Re: NOT ENFORCED constraint feature

On Tue, Dec 10, 2024 at 7:48 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:
>
> >
> > 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.
>
got it.
overall v8-0001 looks good to me!

do you have a patch for
alter check constraint set [not] enforced?
If not, I will probably try to work on it.

I am playing around with the remaining patch.

ATExecAlterConstrRecurse
ATExecAlterConstrEnforceability
ATExecAlterChildConstr
AlterConstrTriggerDeferrability
These four functions take a lot of arguments.
more comments about these arguments would be helpful.
we only need to mention it at ATExecAlterConstrRecurse.

for example:
ATExecAlterConstrRecurse(Constraint *cmdcon, Relation conrel, Relation tgrel,
const Oid fkrelid, const Oid pkrelid,
HeapTuple contuple, List **otherrelids,
LOCKMODE lockmode, Oid ReferencedParentDelTrigger,
Oid ReferencedParentUpdTrigger,
Oid ReferencingParentInsTrigger,
Oid ReferencingParentUpdTrigger)
the comments only explained otherrelids.

LOCKMODE lockmode,
Oid ReferencedParentDelTrigger,
Oid ReferencedParentUpdTrigger,
Oid ReferencingParentInsTrigger,
Oid ReferencingParentUpdTrigger

The above arguments are pretty intuitive.

Constraint *cmdcon
Relation conrel
Relation tgrel
HeapTuple contuple

but these arguments are not that very intuitive,
especially these arguments passing to another function.