Re: pgAdmin III commit: Lots of work on domains, and check constraints

From: Guillaume Lelarge <guillaume(at)lelarge(dot)info>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Timon <timosha(at)gmail(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: pgAdmin III commit: Lots of work on domains, and check constraints
Date: 2012-08-26 16:18:05
Message-ID: 1345997885.5678.12.camel@localhost.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Mon, 2012-07-23 at 08:38 +0100, Dave Page wrote:
> On Sat, Jul 21, 2012 at 1:40 PM, Guillaume Lelarge
> <guillaume(at)lelarge(dot)info> wrote:
> > On Wed, 2012-06-06 at 10:50 +0600, Timon wrote:
> >> seems that this commit broke reindexing of selected index. steps to reproduce:
> >> 1) create table
> >> 2) create index
> >> 3) select index in object inspector
> >> 4) try to reindex it via maintenance menu item
> >> 5) got error : ERROR: schema "table_name" does not exist
> >>
> >> and one more crash here
> >> .. same steps as before
> >> 4) try to CLUSTER index
> >> 5) pgadmin simply crashed
> >>
> >
> > OK, I finally got some time to work on this. As Timon said, these bugs
> > come from the patch "Lots of work on domains, and check constraints". In
> > this patch, I changed some objects parent class from pgTableObject to
> > pgSchemaObject. Due to this change, the GetTable() method returns NULL,
> > which segfaults all statements that try to use the return value without
> > checking. The two examples above from Timon are exactly this.
> >
> > I don't see many ways to get out of this issue.
> >
> > We could use GetSchema() instead of GetTable(). It works, it's an easy
> > and small patch. But it'll certainly be a maintenance nightmare (at
> > least without any comments)
> >
> > We could also revert my patch. It's simple, we loose the feature of
> > adding as many check constraints as we want to a domain, we loose the
> > feature of renaming and validating constraints, and we gain a few bugs.
> >
> > I don't see any other options. My own personal choice would be the first
> > one (see attached patch). But it's a tough call.
>
> We've run into problems in the past every time we've tried to share a
> sub-class between two parents. I think we should stop trying to do
> that, and just resign ourselves to having to duplicate the class - I
> guess pgCheckConstraint and pgDomainCheckConstraint is the way to go.

I don't think I'll have the time and motivation to work on this before
we go GA. I guess I'll have to do this later on but in the mean time,
should I revert my commit or apply this patch?

--
Guillaume
http://blog.guillaume.lelarge.info
http://www.dalibo.com

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Guillaume Lelarge 2012-08-26 16:23:50 Re: Autosave in the SQL Editor
Previous Message Guillaume Lelarge 2012-08-26 13:56:11 pgAdmin III commit: Fix renaming a database