Re: NOT ENFORCED constraint feature

On Tue, Mar 25, 2025 at 10:18 PM Peter Eisentraut <peter(at)eisentraut(dot)org> wrote:
>
> On 21.03.25 06:58, Amul Sul wrote:
> >
> > [....]
> > Attached is the updated version, where the commit messages for patch
> > 0005 and 0006 have been slightly corrected. Additionally, a few code
> > comments have been updated to consistently use the ENFORCED/NOT
> > ENFORCED keywords. The rest of the patches and all the code are
> > unchanged.
>
> I have committed patches 0001 through 0003. I made some small changes:
>

Thank you very much !

> In 0001, I renamed the function UpdateConstraintEntry() to
> AlterConstrUpdateConstraintEntry() so the context is clearer.
>
> In 0002, you had this change:
>
> @@ -12113,75 +12154,89 @@ ATExecAlterConstraintInternal(List **wqueue, ATAlterConstraint *cmdcon,
> * If the table at either end of the constraint is partitioned, we need to
> * handle every constraint that is a child of this one.
> */
> - if (recurse && changed &&
> + if (recurse &&
> (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE ||
> - (OidIsValid(refrelid) &&
> - get_rel_relkind(refrelid) == RELKIND_PARTITIONED_TABLE)))
> - ATExecAlterChildConstr(wqueue, cmdcon, conrel, tgrel, rel, contuple,
> - recurse, otherrelids, lockmode);
> + get_rel_relkind(refrelid) == RELKIND_PARTITIONED_TABLE))
> + AlterConstrDeferrabilityRecurse(wqueue, cmdcon, conrel, tgrel, rel,
> + contuple, recurse, otherrelids,
> + lockmode);
>
> AFAICT, dropping the "changed" from the conditional was not correct. Or at
> least, it would do redundant work if nothing was "changed". So I put that
> back. Let me know if that change was intentional or there is something else
> going on.
>

Makes sense. This is intentional, but I must confess that this change
isn't part of the scope of this patch. I should have mentioned it when
posting, as it was something I intended to discuss with Álvaro, but it
slipped my mind.

The reason for the change is to revert to the behavior before commit
#80d7f990496b1c, where recursion occurred regardless of the
changed flags. This is also described in the header comment for
ATExecAlterConstrDeferrability() (earlier it was for
ATExecAlterConstraintInternal):

*
* Note that we must recurse even when the values are correct, in case
* indirect descendants have had their constraints altered locally.
* (This could be avoided if we forbade altering constraints in partitions
* but existing releases don't do that.)
*

Regards,
Amul