Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> On Sat, 25 Jun 2022 at 04:39, Tom Lane <tgl@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