Dean Rasheed <dean.a.rash...@gmail.com> writes: > On Sat, 25 Jun 2022 at 04:39, Tom Lane <t...@sss.pgh.pa.us> wrote: >> Well, if we want to clean this up a bit rather than just doing the >> minimum safe fix ... I spent some time why we were bothering with the >> FLATCOPY step at all, rather than just mutating the Query* pointer. >> I think the reason is to not fail if the QTW_DONT_COPY_QUERY flag is >> set, but maybe we should clear that flag when recursing?
> Hmm, interesting, but we don't actually pass on that flag when > recursing anyway. Since it is the mutator routine's responsibility to > make a possibly-modified copy of its input node, if it wants to > recurse into the subquery, it should always call query_tree_mutator() > with QTW_DONT_COPY_QUERY unset, and range_table_mutator() should never > need to FLATCOPY() the subquery. Actually, QTW_DONT_COPY_QUERY is dead code AFAICS: we don't use it anywhere, and Debian Code Search doesn't know of any outside users either. Removing it might be something to do in v16. (I think it's a bit late for unnecessary API changes in v15.) > But then, in the interests of further tidying up, why does > range_table_mutator() call copyObject() on the subquery if > QTW_IGNORE_RT_SUBQUERIES is set? I thought about that for a bit, but all of the QTW_IGNORE flags work like that, and I'm hesitant to change it. There may be code that assumes it can modify those trees in-place afterwards. Committed with just the change to use straight MUTATE, making this case exactly like the other places with QTW_IGNORE options. Thanks for the discussion! regards, tom lane