Re: NOT ENFORCED constraint feature

Hi,
I have tested different scenarios involving CHECK constraint with NOT
ENFORCED specification on Inherited and Partitioned tables. Additionally, I
explored various situations with foreign key constraints. I have also
examined how pg_dump, pg_dumpall, pg_basebackup, and pg_upgrade handle NOT
ENFORCED constraints.

On Tue, Dec 17, 2024 at 12:23 PM tushar <tushar(dot)ahuja(at)enterprisedb(dot)com>
wrote:

> FYI
>
> ---------- Forwarded message ---------
> From: Amul Sul <sulamul(at)gmail(dot)com>
> Date: Tue, Dec 10, 2024 at 5:18 PM
> Subject: Re: NOT ENFORCED constraint feature
> To: jian he <jian(dot)universality(at)gmail(dot)com>
> Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Peter Eisentraut <
> peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Joel
> Jacobson <joel(at)compiler(dot)org>
>
>
> On Tue, Dec 10, 2024 at 1:21 PM jian he <jian(dot)universality(at)gmail(dot)com>
> wrote:
> >
> > hi. some minor issue about v7-0001.
> >
> > there are 5 appearances of "sizeof(CookedConstraint)"
> > to make it safe, it would be nice to manual do
> > `
> > cooked->is_enforced = true;
> > `
> > for other kinds of constraints.
> >
>
> I am not sure if it's necessary, but it doesn't seem like a bad idea,
> did the same in the attached version.
>
> >
> > static bool
> > MergeWithExistingConstraint(Relation rel, const char *ccname, Node
> *expr,
> > bool allow_merge, bool is_local,
> > + bool is_enforced,
> > bool is_initially_valid,
> > bool is_no_inherit)
> > {
> > @@ -2729,12 +2738,24 @@ MergeWithExistingConstraint(Relation rel,
> > const char *ccname, Node *expr,
> > * If the child constraint is "not valid" then cannot merge with a
> > * valid parent constraint.
> > */
> > - if (is_initially_valid && !con->convalidated)
> > + if (is_initially_valid && con->conenforced && !con->convalidated)
> > ereport(ERROR,
> > (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> > errmsg("constraint \"%s\" conflicts with NOT VALID constraint on
> > relation \"%s\"",
> > ccname, RelationGetRelationName(rel))));
> >
> > There are no tests for this change. I think this change is not necessary.
> >
>
> It is necessary; otherwise, it would raise an error for a NOT ENFORCED
> constraint, which is NOT VALID by default.
>
> >
> > - a/src/test/regress/expected/alter_table.out
> > +++ b/src/test/regress/expected/alter_table.out
> > ...
> > +ALTER TABLE attmp3 VALIDATE CONSTRAINT b_greater_than_ten_not_enforced;
> -- fail
> > +ERROR: cannot validated NOT ENFORCED constraint
> >
> > there should be
> > ERROR: cannot validate NOT ENFORCED constraint
> > ?
> >
>
> Are you sure you're looking at the latest patch? The error string is
> already the same as you suggested.
>
> > Do we need to update create_foreign_table.sgml
> > and alter_foreign_table.sgml?
>
> Yes, I think we just need to add the syntax; a description isn't
> necessary, imo, since constraints on foreign constraints are never
> enforced.
>
> Regards,
> Amul
>

--
Warm regards,
Triveni