Re: pgAdmin3 and Greenplum partitions SQL

Hi

On Fri, Jan 11, 2013 at 7:31 PM, lpetrov <lppetrov(at)gmail(dot)com> wrote:
> Hi all,
>
> I have identified a bug in handling Greenplum partitions SQL in pgAdmin3.
> The bug is visible when clicking on Greenplum partition table (the partition
> itself) and then the SQL for the table is displayed in the SQL pane on the
> right. The SQL for partitions represents randomly some CREATE TABLE features
> like "UNLOGGED" or "OF " (type).
>
> I have tracked the problem to pgTable and gpPartition. When
> gpPartitionFactory::CreateObjects() creates and inserts the objects, it
> creates gpPartition objects and gpPartition inherits pgTable. Unfortunately
> pgTable does not initialize it's members (is there any reason for that?) and
> relies on the caller/user to set the member variables representing object
> properties one by one.
> With some recent changes introduced in new-ish Postgres versions, new
> members were added to pgTable which were handled in
> pgTableFactory::CreateObjects() (if/else), but not added in
> gpPartitionFactory::CreateObjects(). Therefore when gpPartition object is
> created, members are uninitialized and GetSql() uses random values when
> constructing the SQL string.
>
> This bug can be fixed in several ways, but I believe that the most
> appropriate is to initialize member variables in pgTable. I have attached a
> patch that does this which fixes the bug mentioned above and the entire
> class of similar bugs. Members are initialized in a way to represent the
> default state of table properties (example: table by default is unlogged
> unless explicitly noted, table is not 'of type' unless explicitly noted,
> etc.).
>
> Let me know if you have any comments or remarks.

It looks reasonable from a quick readthrough. Unfortunately the patch
won't apply to either the 1.16 branch or git master - can you
regenerate it against the 1.16 branch using "git diff" please? I don't
have Greenplum available to test, so I'd prefer to be working with a
patch that you've tested rather than manually hacking it about here
and hoping I don't mess it up.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company