Hi, On Fri, Jan 18, 2013 at 12:48 AM, Dave Page <dp...@pgadmin.org> wrote:
> Hi > > > > On Thu, Jan 17, 2013 at 2:56 AM, Lubomir Petrov <lppet...@gmail.com> > wrote: > > On Wed, Jan 16, 2013 at 8:56 AM, Dave Page <dp...@pgadmin.org> wrote: > >> > >> Hi > >> > >> On Fri, Jan 11, 2013 at 7:31 PM, lpetrov <lppet...@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 - it still wouldn't apply though for reasons that weren't > obvious. I've manually applied it and pushed it to the 1.16 and master > branches though - please confirm I didn't break it. > > Thanks, look good! Regards, Lubomir Petrov
