Re: Core dump in range_table_mutator()

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.

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