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

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.

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

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