Re: [badalex@gmail.com: Re: [BUGS] Problem identifying constraints which should not be inherited]

"Alex Hunsaker" <badalex(at)gmail(dot)com> writes:
> [ patch to change inherited-check-constraint behavior ]

Applied after rather heavy editorializations. You didn't do very well on
getting it to work in multiple-inheritance scenarios, such as

create table p (f1 int check (f1>0));
create table c1 (f2 int) inherits (p);
create table c2 (f3 int) inherits (p);
create table cc () inherits (c1,c2);

Here the same constraint is multiply inherited. The base case as above
worked okay, but adding the constraint to an existing inheritance tree
via ALTER TABLE, not so much.

I also didn't like the choice to add is_local/inhcount fields to
ConstrCheck: that struct is fairly heavily used, but you were leaving the
fields undefined/invalid in most code paths, which would surely lead to
bugs down the road when somebody expected them to contain valid data.
I considered extending the patch to always set them up, but rejected that
idea because ConstrCheck is essentially a creature of the executor, which
couldn't care less about constraint inheritance. After some reflection
I chose to put the fields in CookedConstraint instead, which is used only
in the table creation / constraint addition code paths. That required
a bit of refactoring of the API of heap_create_with_catalog, but I think
it ended up noticeably cleaner: constraints and defaults are fed to
heap.c in only one format now.

I found one case that has not really worked as intended for a long time:
ALTER TABLE ADD CHECK ... (that is, ADD CONSTRAINT without specifying
a constraint name) failed to ensure that the same constraint name was used
at child tables as at the parent, and thus the constraints ended up not
being seen as related at all. Fixing this was a bit ugly since it meant
that ADD CONSTRAINT has to recurse to child tables during Phase 2 after
all, and has to be able to add work queue entries for Phase 3 at that
time, which is not something needed by any other ALTER TABLE operation.

I'm not sure if we ought to try to back-patch that --- it'd be a
behavioral change with non-obvious implications. In the back branches,
ADD CHECK followed by DROP CONSTRAINT will end up not deleting the
child-table constraints, which is probably a bug but I wouldn't be
surprised if applications were depending on the behavior.

regards, tom lane