Re: pgAdmin3 and Greenplum partitions SQL - Mailing list pgadmin-hackers

From Lubomir Petrov
Subject Re: pgAdmin3 and Greenplum partitions SQL
Date
Msg-id CA+Z0ECfuQFzy444WtjuA_AJkRHrXurPorJOFrOwF=dCC_8+4sQ@mail.gmail.com
Whole thread Raw
In response to Re: pgAdmin3 and Greenplum partitions SQL  (Dave Page <dpage@pgadmin.org>)
Responses Re: pgAdmin3 and Greenplum partitions SQL  (Dave Page <dpage@pgadmin.org>)
List pgadmin-hackers
On Wed, Jan 16, 2013 at 8:56 AM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Fri, Jan 11, 2013 at 7:31 PM, lpetrov <lppetrov@gmail.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.


Hi,

Sure, attached is the patch against the 1.16 branch generated with "git diff".

Thanks!

Regards,
Lubomir Petrov

Attachment

pgadmin-hackers by date:

Previous
From: Dave Page
Date:
Subject: Re: pgAdmin3 and Greenplum partitions SQL
Next
From: Ian Lawrence Barwick
Date:
Subject: Tiny documentation patch