Hi Amul, On Thu, Feb 27, 2025 at 12:57 AM Amul Sul <sula...@gmail.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