Re: NOT ENFORCED constraint feature

Hi Amul,

On Thu, Feb 27, 2025 at 12:57 AM Amul Sul <sulamul(at)gmail(dot)com> wrote:

> Attached is the rebased patch set against the latest master head,
> which also includes a *new* refactoring patch (0001). In this patch,
> I’ve re-added ATExecAlterChildConstr(), which is required for the main
> feature patch (0008) to handle recursion from different places while
> altering enforceability.

Thanks for the patches!

I reviewed and ran “make check” on each patch. I appreciate how the
patches are organized; separating the refactors from the
implementations made the review process very straightforward.
Overall, LGTM, and I have minor comments below:

0008
Since we are added "convalidated" in some of the constraints tests,
should we also add a "convalidated" field in the "table_constraints"
system view defined in src/backend/catalog/information_schema.sql? If
we do that, we'd also need to update the documentation for this view.

0009
Comment on top of the function ATExecAlterConstrEnforceability():
s/ATExecAlterConstrRecurse/ATExecAlterConstraintInternal/g

Typo in tablecmds.c: s/droping/dropping, s/ke/key
/* We should be droping trigger related to foreign ke constraint */

Thanks,
Alex