From: | Lubomir Petrov <lppetrov(at)gmail(dot)com> |
---|---|
To: | Dave Page <dpage(at)pgadmin(dot)org> |
Cc: | pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org> |
Subject: | Re: pgAdmin3 and Greenplum partitions SQL |
Date: | 2013-01-17 02:56:19 |
Message-ID: | CA+Z0ECfuQFzy444WtjuA_AJkRHrXurPorJOFrOwF=dCC_8+4sQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgadmin-hackers |
On Wed, Jan 16, 2013 at 8:56 AM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> 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.
>
>
Hi,
Sure, attached is the patch against the 1.16 branch generated with "git
diff".
Thanks!
Regards,
Lubomir Petrov
Attachment | Content-Type | Size |
---|---|---|
pgadmin_pgTable.diff | application/octet-stream | 1.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Ian Lawrence Barwick | 2013-01-18 02:42:29 | Tiny documentation patch |
Previous Message | Dave Page | 2013-01-16 16:56:56 | Re: pgAdmin3 and Greenplum partitions SQL |