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