Re: Core dump in range_table_mutator()

On Fri, 24 Jun 2022 at 22:44, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Commit 64919aaab made pull_up_simple_subquery set rte->subquery = NULL
> after doing the deed, so that we don't waste cycles copying a
> now-useless subquery tree around. I discovered today while
> working on another patch that if you invoke query_tree_mutator
> or range_table_mutator on the whole Query after that point,
> range_table_mutator dumps core, because it's expecting subquery
> links to never be NULL. There's apparently noplace in our core
> code that does that today, but I'm a bit surprised we've not heard
> complaints from anyone else. I propose to do this to harden it:
>

Makes sense.

Not directly related to that change ... I think it would be easier to
follow if the CHECKFLATCOPY() was replaced with a separate Assert()
and FLATCOPY() (I had to go and remind myself what CHECKFLATCOPY()
did).

Doing that would allow CHECKFLATCOPY() to be deleted, since this is
the only place that uses it -- every other case knows the node type is
correct before doing a FLATCOPY().

Well almost. The preceding FLATCOPY() of the containing RangeTblEntry
doesn't check the node type, but that could be fixed by using
lfirst_node() instead of lfirst() at the start of the loop, which
would be neater.

Regards,
Dean