Re: Making the subquery alias optional in the FROM clause

On Tue, 5 Jul 2022 at 19:00, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> writes:
> > This was discussed previously in [1], and there seemed to be general
> > consensus in favour of it, but no new patch emerged.
>
> As I said in that thread, I'm not super enthused about this, but I was
> clearly in the minority so I think it should go forward.
>

Cool. Thanks for looking.

> > Attached is a patch that takes the approach of not generating an alias
> > at all, which seems to be neater and simpler, and less code than
> > trying to generate a unique alias.
>
> Hm. Looking at the code surrounding what you touched, I'm reminded
> that we allow JOIN nodes to not have an alias, and represent that
> situation by rte->alias == NULL. I wonder if it'd be better in the
> long run to make alias-less subqueries work similarly,

That is what the patch does: transformRangeSubselect() passes a NULL
alias to addRangeTableEntryForSubquery(), which has been modified to
cope with that in a similar way to addRangeTableEntryForJoin() and
other addRangeTableEntryFor...() functions.

So for example, with the following query, this is what the output from
the parser looks like:

SELECT * FROM (SELECT 1);

query->rtable:
rte:
rtekind = RTE_SUBQUERY
alias = NULL
eref = { aliasname = "subquery", colnames = ... }

> rather than
> generating something that after-the-fact will be indistinguishable
> from a user-written alias. If that turns out to not work well,
> I'd agree with "unnamed_subquery" as the inserted name.
>

The result is distinguishable from a user-written alias, because
rte->alias is NULL. I think the confusion is that when I suggested
using "unnamed_subquery", I was referring to rte->eref->aliasname, and
I still think it's a good idea to change that, for consistency with
unnamed joins.

> Also, what about VALUES clauses? It seems inconsistent to remove
> this restriction for sub-SELECT but not VALUES. Actually it looks
> like your patch already does remove that restriction in gram.y,
> but you didn't follow through elsewhere.
>

It does support unnamed VALUES clauses in the FROM list (there's a
regression test exercising that). It wasn't necessary to make any
additional code changes because addRangeTableEntryForValues() already
supported having a NULL alias, and it all just flowed through.

In fact, the grammar forces you to enclose a VALUES clause in the FROM
list in parentheses, so this ends up being an unnamed subquery in the
FROM list as well. For example:

SELECT * FROM (VALUES(1),(2),(3));

produces

query->rtable:
rte:
rtekind = RTE_SUBQUERY
alias = NULL
eref = { aliasname = "subquery", colnames = ... }
subquery->rtable:
rte:
rtekind = RTE_VALUES
alias = NULL
eref = { aliasname = "*VALUES*", colnames = ... }

So it's not really any different from a normal subquery.

> As far as the docs go, I think it's sufficient to mention the
> inconsistency with SQL down at the bottom; we don't need a
> redundant in-line explanation.

OK, fair enough.

I'll post an update in a little while, but first, I found a bug, which
revealed a pre-existing bug in transformLockingClause(). I'll start a
new thread for that, since it'd be good to get that resolved
independently of this patch.

Regards,
Dean