Re: NOT ENFORCED constraint feature

On Thu, Dec 5, 2024 at 4:40 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> On 2024-Dec-03, Peter Eisentraut wrote:
>
> > The handling of merging check constraints seems incomplete. What
> > should be the behavior of this:
> >
> > => create table p1 (a int check (a > 0) not enforced);
> > CREATE TABLE
> > => create table c1 (a int check (a > 0) enforced) inherits (p1);
> > CREATE TABLE
>
> Hmm. Because the constraints are unnamed, and the chosen names are
> different, I don't think they should be merged; I tried with 0001 in
> place, and I think it does the right thing. If c1's creation specifies
> a name that matches the parent name, we get this:
>
> 55432 18devel 61349=# create table c1 (a int constraint p1_a_check check (a > 0)) inherits (p1);
> NOTICE: merging column "a" with inherited definition
> ERROR: constraint "p1_a_check" conflicts with NOT VALID constraint on relation "c1"
>
> I think this is bogus on two counts. First, NOT VALID has nowhere been
> specified, so the error shouldn't be about that. But second, the child
> should have the constraint marked as enforced as requested, and marked
> as conislocal=t, coninhcount=1; the user can turn it into NOT ENFORCED
> if they want, and no expectation breaks, because the parent is also
> already marked NOT ENFORCED.
>
> The other way around shall not be accepted: if the parent has it as
> ENFORCED, then the child is not allowed to have it as NOT ENFORCED,
> neither during creation nor during ALTER TABLE. The only way to mark
> c1's constraint as NOT ENFORCED is to mark p1's constraint as NOINHERIT,
> so that c1's constraint's inhcount becomes 0. Then, the constraint has
> no parent with an enforced constraint, so it's okay to mark it as not
> enforced.
>

Makes sense, agreed.

> > Or this?
> >
> > => create table p2 (a int check (a > 0) enforced);
> > CREATE TABLE
> > => create table c2 () inherits (p1, p2);
> > CREATE TABLE
> >
> > Should we catch these and error?
>
> Here we end up with constraints p1_a_check and p2_a_check, which have
> identical definitions except the NOT ENFORCED bits differ. I think this
> is okay, since we don't attempt to match these constraints when the
> names differ. If both parents had the constraint with the same name, we
> should try to consider them as one and merge them. In that case, c2's
> constraint inhcount should be 2, and at least one of the parent
> constraints is marked enforced, so the child shall have it as enforce
> also. Trying to mark c2's constraint as NOT ENFORCED shall give an
> error because it inherits from p2. But if you deinherit from p2, or
> mark the constraint in p2 as NOINHERIT, then c2's constraint can become
> NOT ENFORCE if the user asks for it.
>

Agreed to this as well. I have made the changes to align with the
suggested behavior in the attached version. Thank you.

Regards,
Amul