Re: Alias of VALUES RTE in explain plan

Yasir <yasir(dot)hussain(dot)shah(at)gmail(dot)com> writes:
> I have fixed the code to produce desired output by adding a few lines in
> pull_up_simple_subquery().
> Attached patch is divided in 2 files:
> - 001-Fix-Alias-VALUES-RTE.patch contains the actual fix.
> - 002-Fix-Alias-VALUES-RTE.patch contains the expected output changes
> against the actual fix.

I was initially skeptical about this, because we've been printing
"*VALUES*" for a decade or more and there have been few complaints.
So I wondered if the change would annoy more people than liked it.
However, after looking at the output for awhile, it is nice that the
columns of the VALUES are referenced with their user-given names
instead of "columnN". I think that's enough of an improvement that
it'll win people over.

However ... I don't like this implementation, not even a little
bit. Table/column aliases are assigned by the parser, and the
planner has no business overriding them. Quite aside from being
a violation of system structural principles, there are practical
reasons not to do it like that:

1. We'd see different results when considering plan trees than
unplanned query trees.

2. At the place where you put this, some planning transformations have
already been done, and that affects the results. That means that
future extensions or restructuring of the planner might change the
results, which seems undesirable.

I think the right way to make this happen is for the parser to
do it, which it can do by passing down the outer query level's
Alias to addRangeTableEntryForValues. There's a few layers of
subroutine calls between, but we can minimize the pain by adding
a ParseState field to carry the Alias. See attached.

My point 2 is illustrated by the fact that my patch produces
different results in a few cases than yours does --- look at
groupingsets.out in particular. I think that's fine, and
the changes that yours makes and mine doesn't look a little
unprincipled. For example, in the tests involving the "gstest1"
view, if somebody wants nice labels on that view's VALUES columns
then the right place to apply those labels is within the view.
Letting a subsequent call of the view inject labels seems pretty
action-at-a-distance-y.

regards, tom lane