Richard Guo <guofenglinux@gmail.com> writes:
> I think the problem is that when we see a Var that references a
> SUBQUERY RTE when deparsing a Plan tree to get the name of a field, we
> assume that we are in a SubqueryScan plan node, in which case the code
> is no problem because set_deparse_plan has set dpns->inner_plan to its
> child plan. However, this bug shows that this assumption does not
> always hold: we might instead be in a Result node with a Var
> referencing a SUBQUERY RTE.
Yeah. I think the comment about that in get_name_for_var_field was
accurate when written, but that was a few rounds of planner
improvements ago. I found that your simplified query works up to
9.5 but fails in >= 9.6, whereupon I bisected to find
3fc6e2d7f5b652b417fa6937c34de2438d60fa9f is the first bad commit
commit 3fc6e2d7f5b652b417fa6937c34de2438d60fa9f
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon Mar 7 15:58:22 2016 -0500
Make the upper part of the planner work by generating and comparing Paths.
Immediately before that, the non-VERBOSE description of the plan was
Result
One-Time Filter: false
-> Result
while that commit changed it to
Result
One-Time Filter: false
So even before that, we'd been emitting a Result not a SubqueryScan
plan node, but it worked anyway because the lower Result had the
same tlist as the SubqueryScan it replaced: the VERBOSE output was
Result
Output: ((information_schema._pg_expandarray('{1,2}'::integer[]))).x,
((information_schema._pg_expandarray('{1,2}'::integer[]))).n
One-Time Filter: false
-> Result
Output: information_schema._pg_expandarray('{1,2}'::integer[])
However, once we decided we didn't really need the child plan node at
all, get_name_for_var_field was broken. I'm surprised it took this
long to notice.
> ... That is to say, we neither have a valid
> rte->subquery nor a valid SubqueryScan plan node. So it seems to me
> that there is no easy way to get the names of the fields in this case.
Indeed.
> I'm wondering whether we can just compose a fake name with something
> like below?
This seems like a band-aid. Also, I experimented with it and found
that for your test query, the output is
Result
Output: (a).col1, (a).col2
One-Time Filter: false
This is confusing (where'd "a" come from?) and it makes me quite
nervous that there are other cases where we'd also fail. What
we basically have here is a dangling-reference Var with no valid
referent. That's wrong in itself and it seems like it risks
causing executor problems not only EXPLAIN problems. It's just
minimally safe because we know that the tlist will never actually
get evaluated ... but this could easily trip up logic that runs
during executor startup.
I wonder if we shouldn't change what the planner puts into the tlist
of a dummy Result. That is, I'm imagining reducing the tlist of
such a Result to null Consts that would serve to show the right column
datatypes and not much else:
Result
Output: NULL, NULL
One-Time Filter: false
I've not looked at how messy this might get, though.
> This same problem can also happen to CTEs.
Yeah, basically the same thing in the RTE_CTE switch case.
regards, tom lane
