On Thu, Feb 27, 2025 at 4:48 PM Álvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2025-Feb-27, Amul Sul 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.
>
> I think you refer to ATExecAlterConstrEnforceability, which claims
> (falsely) that it is a subroutine to ATExecAlterConstrRecurse; in
> reality it is called from ATExecAlterConstraintInternal or at least
> that's what I see in your 0008. So I wonder if you haven't confused
> yourself here. If nothing else, that comments needs fixed. I didn't
> review these patches.
>
Yeah, that was intentional. I wanted to avoid recursion again by
hitting ATExecAlterChildConstr() at the end of
ATExecAlterConstraintInternal(). Also, I realized the value doesn’t
matter since recurse = false is explicitly set inside the
cmdcon->alterEnforceability condition. I wasn’t fully satisfied with
how we handled the recursion decision (code design), so I’ll give it
more thought. If I don’t find a better approach, I’ll add clearer
comments to explain the reasoning.
Regards,
Amul
