Re: enable/disable broken for statement triggers on partitioned tables

On Thu, Aug 4, 2022 at 3:01 AM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> On 2022-Aug-02, Amit Langote wrote:
> > Regarding the patch, I agree that storing the recurse flag rather than
> > overwriting subtype might be better.
> >
> > + bool execTimeRecursion; /* set by ATPrepCmd if ATExecCmd must
> > + * recurse to children */
> >
> > Might it be better to call this field simply 'recurse'? I think it's
> > clear from the context and the comment above the flag is to be used
> > during execution.
>
> Yeah, I guess we can do that and also reword the overall ALTER TABLE
> comment about recursion. That's in the attached first patch, which is
> intended as backpatchable.

Thanks. This one looks good to me.

> The second patch is just to show how we'd rewrite AT_AddColumn to no
> longer use the Recurse separate enum value but instead use the ->recurse
> flag. This is pretty straightforward and it's a clear net reduction of
> code. We can't backpatch this kind of thing of course, both because of
> the ABI break (easily fixed) and because potential destabilization
> (scary). We can do similar tihngs for the other AT enum values for
> recursion. This isn't complete since there are a few other values in
> that enum that we should process in this way too; I don't intend it to
> push it just yet.

I like the idea of removing all AT_*Recurse subtypes in HEAD.

--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com