Horton

Re: NOT ENFORCED constraint feature

amul sul <sulamul@gmail.com>

View thread
From: Amul Sul <sulamul@gmail.com>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: Álvaro Herrera <alvherre@alvh.no-ip.org>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, jian he <jian.universality@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Joel Jacobson <joel@compiler.org>, Alexandra Wang <alexandra.wang.oss@gmail.com>, Suraj Kharage <suraj.kharage@enterprisedb.com>
Date: 2025-03-26T04:02:33Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add support for NOT ENFORCED in foreign key constraints

  2. Expand test a bit

  3. refactor: Pass relation OID instead of Relation to createForeignKeyCheckTriggers()

  4. refactor: Split ATExecAlterConstraintInternal()

  5. refactor: Move some code that updates pg_constraint to a separate function

  6. Move RemoveInheritedConstraint() call slightly earlier

  7. refactor: Split tryAttachPartitionForeignKey()

  8. refactor: re-add ATExecAlterChildConstr()

  9. Add ATAlterConstraint struct for ALTER .. CONSTRAINT

  10. refactor: split ATExecAlterConstrRecurse()

  11. Add support for NOT ENFORCED in CHECK constraints

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