Re: Alias of VALUES RTE in explain plan

Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> writes:
> On Tue, Oct 29, 2024 at 10:49 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> So what I now think
>> is that we ought to tweak the patch so that the parent alias is
>> pushed down only when the subquery contains just VALUES, no other
>> clauses. Per a look at the grammar, ORDER BY, LIMIT, and FOR
>> UPDATE could conceivably appear alongside VALUES; although
>> FOR UPDATE would draw "FOR UPDATE cannot be applied to VALUES",
>> so maybe we needn't worry about it.

> If the user writes it in this manner, I think they intend to attach
> the alias to VALUES() since there's no other way to do it. What is
> weird is that they can use the alias before it's declared. For the
> sake of eliminating this weirdness, your proposed tweak sounds fine to
> me.

I was starting to come around to Andrei's position that changing this
behavior is fine, until I realized that it creates a problem for
ruleutils.c. With the v2 patch, dumping a view that contains a
construct like this doesn't work:

regression=# create view vv as SELECT * FROM (VALUES (4),(2),(3),(1) ORDER BY t1.x LIMIT 2) AS t1(x);
CREATE VIEW
regression=# \d+ vv
View "public.vv"
Column | Type | Collation | Nullable | Default | Storage | Description
--------+---------+-----------+----------+---------+---------+-------------
x | integer | | | | plain |
View definition:
SELECT x
FROM ( VALUES (4), (2), (3), (1)
ORDER BY t1_1.x
LIMIT 2) t1(x);

ruleutils has decided that it needs to make the two "t1" table
aliases distinct. But of course that will fail on reload:

regression=# SELECT x
regression-# FROM ( VALUES (4), (2), (3), (1)
regression(# ORDER BY t1_1.x
regression(# LIMIT 2) t1(x);
ERROR: missing FROM-clause entry for table "t1_1"
LINE 3: ORDER BY t1_1.x
^

Now maybe we could teach ruleutils that these table aliases don't have
to be distinct. But that feels fragile, and it's work that we'd be
expending only so that we can break any existing SQL code that's
using this construct. That's enough to put me firmly on the side of
"let's not change that behavior".

It seems sufficient to avoid alias pushdown when there's an ORDER BY
inside the VALUES subquery. We disallow a locking clause, and
while there can be LIMIT/OFFSET, those aren't allowed to reference the
VALUES output anyway. I added some test cases to show that this is
enough to make view-dumping behave sanely.

regards, tom lane