On Thu, Feb 27, 2025 at 4:48 PM Álvaro Herrera <alvhe...@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
