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.
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? If QTW_IGNORE_RT_SUBQUERIES isn't
set, the mutator routine will either copy and modify the subquery, or
it will return the original unmodified subquery node via
expression_tree_mutator(), without copying it. So then if
QTW_IGNORE_RT_SUBQUERIES is set, why not also just return the original
unmodified subquery node?
So then the RTE_SUBQUERY case in range_table_mutator() would only have to do:
case RTE_SUBQUERY:
if (!(flags & QTW_IGNORE_RT_SUBQUERIES))
MUTATE(newrte->subquery, newrte->subquery, Query *);
break;
which wouldn't fall over if the subquery were NULL.
Regards,
Dean
