Re: NOT ENFORCED constraint feature - Mailing list pgsql-hackers

On Tue, Mar 25, 2025 at 10:18 PM Peter Eisentraut <peter@eisentraut.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