Re: NOT ENFORCED constraint feature

On Tue, Dec 3, 2024 at 6:29 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 03.12.24 13:00, Amul Sul wrote:
> [....]
> >
> > Ok. Initially, I was doing it the same way, but to maintain consistency
> > with the pg_constraint column and avoid negation in multiple places, I
> > chose that approach. However, I agree that having the default to false
> > would be safer. I’ve renamed the flag to is_not_enforced. Other names
> > I considered were not_enforced or is_unenforced, but since we already
> > have existing flags with two underscores, is_not_enforced shouldn't be
> > a problem.
>
> I was initially thinking about this as well, but after seeing it now, I
> don't think this is a good change.  Because now we have both "enforced"
> and "not_enforced" sprinkled around the code.  If we were to do this
> consistently everywhere, then it might make sense, but this way it's
> just confusing.  The Constraint struct is only initialized in a few
> places, so I think we can be careful there.  Also note that the field
> initially_valid is equally usually true.
>

Ok, reverted and returned to using the is_enforced flag as before.

> I could of other notes on patch 0001:
>
> Update information_schema table_constraint.enforced (see
> src/backend/catalog/information_schema.sql and
> doc/src/sgml/information_schema.sgml).
>

Done.

> 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
>
> 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?
>

I don't see any issue with treating ENFORCED and NOT ENFORCED as
distinct constraints if the names are different. However, if the names
are the same but the enforceability differs, I think we should throw
an error for now.

A better implementation I can think of would be to have behavior
similar to the NULL constraint: if the same column in one parent is
NOT NULL and another is not, the inherited child should always have
the NOT NULL column constraint, (in our case it should be ENFORCED),
eg: in the following case, c2 will have the column set as NOT NULL.

create table p1 (a int NOT NULL);
create table p2 (a int);
create table c2 () inherits (p1, p2);

But, I am a bit skeptical about following where it gets NOT NULL as
well but I expected it shouldn't be, is that right behaviour?
create table c3 (a int NULL) inherits (p1, p2);

So, if we want the child constraint enforceability to be overridden by
the child specification, we should handle it accordingly, unlike the
above NULL case. Thoughts?

Attached is the updated version that includes fixes based on Jian He's
review comments. 0005 is still WIP for the same reasons mentioned in
the v5 version. Alvaro also confirmed to me of-list, that the NOT
VALID FK constraint hasn't been implemented, and simply dropping the
restriction (the error check) may not be sufficient for its
implementation. I still need to investigate what else is required,
which is why this version remains WIP.

Regards,
Amul