On Thu, Aug 4, 2022 at 3:01 AM Alvaro Herrera <alvhe...@alvh.no-ip.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
