Обсуждение: [HACKERS] Make subquery alias optional in FROM clause

From time to time, especially during migration projects from Oracle to
PostgreSQL, i'm faced with people questioning why the alias in the FROM
clause for subqueries in PostgreSQL is mandatory. The default answer
here is, the SQL standard requires it.

This also is exactly the comment in our parser about this topic:

/*
 * The SQL spec does not permit a subselect
 * (<derived_table>) without an alias clause,
 * so we don't either.  This avoids the problem
 * of needing to invent a unique refname for it.
 * That could be surmounted if there's sufficient
 * popular demand, but for now let's just implement
 * the spec and see if anyone complains.
 * However, it does seem like a good idea to emit
 * an error message that's better than "syntax error".
 */

So i thought i'm the one standing up for voting to relax this and
making the alias optional.

The main problem, as mentioned in the parser's comment, is to invent a
machinery to create an unique alias for each of the subquery/values
expression in the from clause. I pondered a little about it and came to
the attached result.

The patch generates an auto-alias for subqueries in the format
*SUBQUERY_<RTI>* for subqueries and *VALUES_<RTI>* for values
expressions. <RTI> is the range table index it gets during
transformRangeSubselect().

Doc patch and simple regression tests included.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Bernd Helmle <mailings@oopsware.de> writes:
>> From time to time, especially during migration projects from Oracle to
> PostgreSQL, i'm faced with people questioning why the alias in the FROM
> clause for subqueries in PostgreSQL is mandatory. The default answer
> here is, the SQL standard requires it.

Indeed.  When I wrote the comment you're referring to, quite a few years
ago now, I thought that popular demand might force us to allow omitted
aliases.  But the demand never materialized.  At this point it seems
clear to me that there isn't really good reason to exceed the spec here.
It just encourages people to write unportable SQL code.

> The patch generates an auto-alias for subqueries in the format
> *SUBQUERY_<RTI>* for subqueries and *VALUES_<RTI>* for values
> expressions. <RTI> is the range table index it gets during
> transformRangeSubselect().

This is not a solution, because it does nothing to avoid conflicts with
table names elsewhere in the FROM clause.  If we were going to relax this
--- which, I repeat, I'm against --- we'd have to come up with something
that would thumb through the whole query and make sure what it was
generating didn't already appear somewhere else.  Or else not generate
a name at all, in which case there simply wouldn't be a way to refer to
the subquery by name; I'm not sure what that might break though.
        regards, tom lane

On Wed, 2017-02-22 at 10:08 -0500, Tom Lane wrote:
> 
> Indeed.  When I wrote the comment you're referring to, quite a few
> years
> ago now, I thought that popular demand might force us to allow
> omitted
> aliases.  But the demand never materialized.  At this point it seems
> clear to me that there isn't really good reason to exceed the spec
> here.
> It just encourages people to write unportable SQL code.
> 

Years ago i didn't hear anything about it either. But in the last few
months i've heard such a demand several times, so i thought we should
give it another try.

> > The patch generates an auto-alias for subqueries in the format
> > *SUBQUERY_<RTI>* for subqueries and *VALUES_<RTI>* for values
> > expressions. <RTI> is the range table index it gets during
> > transformRangeSubselect().
> 
> This is not a solution, because it does nothing to avoid conflicts
> with
> table names elsewhere in the FROM clause.  If we were going to relax
> this
> --- which, I repeat, I'm against --- we'd have to come up with
> something
> that would thumb through the whole query and make sure what it was
> generating didn't already appear somewhere else.  

I've thought about this already. One thing that came into my mind was
to maintain a lookup list of aliasnames during the transform phase and
throw an ereport as soon as the generated string has any duplicate. Not
sure about the details, but i was worried about the performance impact
in this area...

> Or else not generate
> a name at all, in which case there simply wouldn't be a way to refer
> to
> the subquery by name; I'm not sure what that might break though.
> 

Hmm, maybe that's an option. Though, i think parts of the code aren't
prepared to deal with empty (or even NULL) aliases. That's likely much
more invasive.

On Wed, Feb 22, 2017 at 10:08:38AM -0500, Tom Lane wrote:
> Bernd Helmle <mailings@oopsware.de> writes:
> >> From time to time, especially during migration projects from Oracle to
> > PostgreSQL, i'm faced with people questioning why the alias in the FROM
> > clause for subqueries in PostgreSQL is mandatory. The default answer
> > here is, the SQL standard requires it.
> 
> Indeed.  When I wrote the comment you're referring to, quite a few years
> ago now, I thought that popular demand might force us to allow omitted
> aliases.  But the demand never materialized.  At this point it seems
> clear to me that there isn't really good reason to exceed the spec here.
> It just encourages people to write unportable SQL code.

I suspect most users, like me, just roll their eyes, grumble, and put up
with it rather than complain.  It's a pain point, but tolerable enough
that no one bothers to demand a change.  Now that it's been done though,
allow me to add my voice in favor of it!

> > The patch generates an auto-alias for subqueries in the format
> > *SUBQUERY_<RTI>* for subqueries and *VALUES_<RTI>* for values
> > expressions. <RTI> is the range table index it gets during
> > transformRangeSubselect().
> 
> This is not a solution, because it does nothing to avoid conflicts with
> table names elsewhere in the FROM clause.  If we were going to relax this
> --- which, I repeat, I'm against --- we'd have to come up with something
> that would thumb through the whole query and make sure what it was
> generating didn't already appear somewhere else.  Or else not generate
> a name at all, in which case there simply wouldn't be a way to refer to
> the subquery by name; I'm not sure what that might break though.

On alias conflict... backtrack and retry with a new set of sub-query
names.  For generating the alias names all you need is a gensym-style
counter.  But yes, even this is tricky because you'd have to check that
the conflicting alias name is one of the gensym'ed ones.

Nico
-- 

On Wed, 2017-02-22 at 08:13 -0700, David G. Johnston wrote:
> I'll contribute to the popular demand aspect but given that the error
> is
> good and the fix is very simple its not exactly a strong desire.

In one project i've recently seen, for some reasons, they need to
maintain an application twice, one for Oracle and the other for
Postgres for years. To be honest, subqueries aren't the only problem,
but having this solved in the backend itself would help to decrease the
amount of maintenance efforts in such projects.

I thought that this would be another thing to make the migration pains
more less, without being too invasive, given that there were already
some thoughts about relaxing alias usage.

On Wed, Feb 22, 2017 at 10:33 PM, Nico Williams <nico@cryptonector.com> wrote:
> On Wed, Feb 22, 2017 at 10:08:38AM -0500, Tom Lane wrote:
>> Bernd Helmle <mailings@oopsware.de> writes:
>> >> From time to time, especially during migration projects from Oracle to
>> > PostgreSQL, i'm faced with people questioning why the alias in the FROM
>> > clause for subqueries in PostgreSQL is mandatory. The default answer
>> > here is, the SQL standard requires it.
>>
>> Indeed.  When I wrote the comment you're referring to, quite a few years
>> ago now, I thought that popular demand might force us to allow omitted
>> aliases.  But the demand never materialized.  At this point it seems
>> clear to me that there isn't really good reason to exceed the spec here.
>> It just encourages people to write unportable SQL code.
>
> I suspect most users, like me, just roll their eyes, grumble, and put up
> with it rather than complain.  It's a pain point, but tolerable enough
> that no one bothers to demand a change.  Now that it's been done though,
> allow me to add my voice in favor of it!

+1 to all of that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2017-02-23 08:21:41 +0530, Robert Haas wrote:
> On Wed, Feb 22, 2017 at 10:33 PM, Nico Williams <nico@cryptonector.com> wrote:
> > On Wed, Feb 22, 2017 at 10:08:38AM -0500, Tom Lane wrote:
> >> Bernd Helmle <mailings@oopsware.de> writes:
> >> >> From time to time, especially during migration projects from Oracle to
> >> > PostgreSQL, i'm faced with people questioning why the alias in the FROM
> >> > clause for subqueries in PostgreSQL is mandatory. The default answer
> >> > here is, the SQL standard requires it.
> >>
> >> Indeed.  When I wrote the comment you're referring to, quite a few years
> >> ago now, I thought that popular demand might force us to allow omitted
> >> aliases.  But the demand never materialized.  At this point it seems
> >> clear to me that there isn't really good reason to exceed the spec here.
> >> It just encourages people to write unportable SQL code.
> >
> > I suspect most users, like me, just roll their eyes, grumble, and put up
> > with it rather than complain.  It's a pain point, but tolerable enough
> > that no one bothers to demand a change.  Now that it's been done though,
> > allow me to add my voice in favor of it!
> 
> +1 to all of that.

+1, too.

Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Feb 22, 2017 at 10:33 PM, Nico Williams <nico@cryptonector.com> wrote:
>> I suspect most users, like me, just roll their eyes, grumble, and put up
>> with it rather than complain.  It's a pain point, but tolerable enough
>> that no one bothers to demand a change.  Now that it's been done though,
>> allow me to add my voice in favor of it!

> +1 to all of that.

[ shrug... ]  Well, I won't resist this hard as long as it's done
competently, which to me means "the subquery name doesn't conflict with
anything else".  Not "it doesn't conflict unless you're unlucky enough
to have used the same name elsewhere".  There are a couple ways we could
achieve that result, but the submitted patch fails to.

(Or, in words of one syllable: if I thought this way was okay, I would
have done it like that back in 2000.)
        regards, tom lane

Am Mittwoch, den 22.02.2017, 22:17 -0500 schrieb Tom Lane:
> [ shrug... ]  Well, I won't resist this hard as long as it's done
> competently, which to me means "the subquery name doesn't conflict
> with
> anything else".  Not "it doesn't conflict unless you're unlucky
> enough
> to have used the same name elsewhere".  There are a couple ways we
> could
> achieve that result, but the submitted patch fails to.

Right, i'm going to give it a try then. Currently i see these options:

* Validate any generated alias against a list of explicit alias names.

This means we have to collect explicit alias names in, say a hashtable,
and validate a generated name against potential collisions and retry.
Or better, generate the name in a way that doesn't produce a collision
with this list.

* Don't force/generate an alias at all.

I've no idea for this yet and Tom already was concerned what this might
break. There are several places in the transform phase where the
refnames are required (e.g. isLockedRefname()).


Thanks
Bernd

On 22 February 2017 at 15:08, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Indeed.  When I wrote the comment you're referring to, quite a few years
> ago now, I thought that popular demand might force us to allow omitted
> aliases.  But the demand never materialized.  At this point it seems
> clear to me that there isn't really good reason to exceed the spec here.
> It just encourages people to write unportable SQL code.


Oh my. This bothers me all the time. I always assumed the reason it
was like this was because the grammar would be ambiguous without it
and it would require extreme measures to hack the grammar to work. If
it's this easy I would totally be for it.

Offhand I think there are plenty of solutions for the problem of
inventing names and I suspect any of them would work fine:

1) Don't assign a name -- I would guess this would require some
adjustments in the rule deparsing (i.e. views).

2) Assign a name but add a flag indicating the name is autogenerated
and shouldn't be used for resolving references and shouldn't be
dumped. Then it shouldn't really matter if there's a conflict since
the name is only used for things like error messages, not resolving
references.

3) thumb through all the names in the query and pick one that doesn't conflict.

For what it's worth while it wouldn't be a *bad* thing to avoid
conflicts I think this is being held to an inconsistent standard here.
It's not like there aren't similar situations elsewhere in the
codebase where we just don't worry about this kind of thing:

=> SELECT "?column"? FROM (select 1+1 as "?column?", 1+1) AS x;
ERROR:  42703: column "?column" does not exist
LINE 2: SELECT "?column"? FROM (select 1+1 as "?column?", 1+1) AS x;              ^
HINT:  Perhaps you meant to reference the column "x.?column?" or the
column "x.?column?".

-- 
greg

On 23 February 2017 at 13:27, Greg Stark <stark@mit.edu> wrote:
> => SELECT "?column"? FROM (select 1+1 as "?column?", 1+1) AS x;

Oops, I missed the typo there:

=> SELECT "?column?" FROM (select 1+1 AS "?column?", 1+1) AS x;
ERROR:  42702: column reference "?column?" is ambiguous
LINE 2: SELECT "?column?" FROM (select 1+1 AS "?column?", 1+1) AS x;              ^
LOCATION:  scanRTEForColumn, parse_relation.c:669


-- 
greg

Greg Stark <stark@mit.edu> writes:
> On 22 February 2017 at 15:08, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Indeed.  When I wrote the comment you're referring to, quite a few years
>> ago now, I thought that popular demand might force us to allow omitted
>> aliases.  But the demand never materialized.  At this point it seems
>> clear to me that there isn't really good reason to exceed the spec here.
>> It just encourages people to write unportable SQL code.

> For what it's worth while it wouldn't be a *bad* thing to avoid
> conflicts I think this is being held to an inconsistent standard here.

True, but there are reasons for it:

1. We don't insist on column names in a SQL query being unique.
We do insist on table aliases being unique.  So a name generation
rule for table aliases that fails to ensure uniqueness will result
in duplicate-alias failures, where the same doesn't happen for
non-unique column aliases unless you try to reference them.  (BTW,
I would argue that a query that leaves an alias undetermined, and then
tries to reference the column or table by name anyway, is broken by
design.  So I'm unimpressed by any complaints based on that scenario.)

2. Our standards are higher than they were twenty years ago.  Somebody
who submitted the current approach to generating column aliases today
would likely get laughed off the mailing list.  I doubt it's worth the
costs of changing it, but that doesn't mean I'm prepared to adopt an
equally sloppy solution in a place where the stakes are higher.
        regards, tom lane

Bernd Helmle <mailings@oopsware.de> writes:
> Right, i'm going to give it a try then. Currently i see these options:

> * Validate any generated alias against a list of explicit alias names.

> This means we have to collect explicit alias names in, say a hashtable,
> and validate a generated name against potential collisions and retry.

TBH I think a hashtable is overkill.  You could just run through the FROM
clause and see what's there (compare what happens in ruleutils.c when it
has to invent an alias).  The difficulty is not so much with time taken
as that it's not very clear how to manage things when the querytree is
only half-transformed.  You don't really want this code having to deal
with both transformed and untransformed querytrees, much less knowing
which parts are where.

Maybe the best answer is to not do it immediately when transforming the
subselect's RTE, but to go back after we've finished transforming the
entire FROM clause and add aliases to any RTEs that lack them.  I think
probably at that point you could just examine the RTE list and not worry
about any jointree data structures at all.  It might be worth adding a
flag to ParseState to remember whether any aliases require assignment, so
that you could skip looking in the standard case.

> * Don't force/generate an alias at all.

> I've no idea for this yet and Tom already was concerned what this might
> break. There are several places in the transform phase where the
> refnames are required (e.g. isLockedRefname()).

Yeah.  This would be cleaner in some sense but also a lot more delicate.
Not sure it's worth the trouble.
        regards, tom lane

On Thu, Feb 23, 2017 at 10:37:16AM +0100, Bernd Helmle wrote:
> Am Mittwoch, den 22.02.2017, 22:17 -0500 schrieb Tom Lane:
> > [ shrug... ]  Well, I won't resist this hard as long as it's done
> > competently, which to me means "the subquery name doesn't conflict
> > with
> > anything else".  Not "it doesn't conflict unless you're unlucky
> > enough
> > to have used the same name elsewhere".  There are a couple ways we
> > could
> > achieve that result, but the submitted patch fails to.
>
> Right, i'm going to give it a try then. Currently i see these options:
>
> * Validate any generated alias against a list of explicit alias names.
>
> This means we have to collect explicit alias names in, say a hashtable,
> and validate a generated name against potential collisions and retry.
> Or better, generate the name in a way that doesn't produce a collision
> with this list.

There's another option:
* Gensym an alias name, and if the compilation fails with that alias  name as a conflict, try again with a new
gensym'edname. 

> * Don't force/generate an alias at all.

That seems like a lot of work.

Nico
--

On Thu, Feb 23, 2017 at 7:50 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Maybe the best answer is to not do it immediately when transforming the
> subselect's RTE, but to go back after we've finished transforming the
> entire FROM clause and add aliases to any RTEs that lack them.  I think
> probably at that point you could just examine the RTE list and not worry
> about any jointree data structures at all.  It might be worth adding a
> flag to ParseState to remember whether any aliases require assignment, so
> that you could skip looking in the standard case.

Right, something along these lines makes sense to me.  If we try to
generate an alias before we know what all the things that it might
later conflict with are, then somebody will always be able to write a
query where it breaks.

>> I've no idea for this yet and Tom already was concerned what this might
>> break. There are several places in the transform phase where the
>> refnames are required (e.g. isLockedRefname()).
>
> Yeah.  This would be cleaner in some sense but also a lot more delicate.
> Not sure it's worth the trouble.

It sounds like a lot of trouble.  It does also sound nicer.  I suppose
whether it's worth the trouble is for the person doing the work to
decide.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Feb 23, 2017 at 4:08 PM, Pantelis Theodosiou <ypercube@gmail.com> wrote:
> Question: Will the patch be removed if and when Oracle decides to be
> compatible with the standard and forbids non-aliased derived tables?
>
> (I know it's a rather theoretical question. Unlikely that Oracle breaks
> backwards compatibility on that.)

Even if they did, so what?

First of all, our project's aim is not to copy Oracle slavishly but to
build a good database.  Sometimes that involves making things work in
ways similar to Oracle and sometimes it doesn't.  For example, I have
no urge to get rid of transactional DDL just because Oracle doesn't
have it.  I have no feeling that NULL should behave in the completely
unprincipled way that it does in Oracle.  And I don't think that
PostGIS needs to try to go be more like Oracle Spatial.

Secondly, extensions to the standard that let reasonable things work
which the standard doesn't permit are generally a good idea.  We don't
want to let things work that really deserve to fail - for example
because the meaning is ambiguous - nor do we want to implement
standard syntax with non-standard semantics.  However, neither of
those problems exists for this case.  I don't see the point in making
things fail that could just as easily do what was wanted; that seems
pedantic.  I don't think it's only Oracle that allows omitting the
alias; I think there are a number of other systems that behave
similarly.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 23 February 2017 at 22:20, Tom Lane <tgl@sss.pgh.pa.us> wrote:

>> * Don't force/generate an alias at all.
>
>> I've no idea for this yet and Tom already was concerned what this might
>> break. There are several places in the transform phase where the
>> refnames are required (e.g. isLockedRefname()).
>
> Yeah.  This would be cleaner in some sense but also a lot more delicate.
> Not sure it's worth the trouble.

Personally I think we need to generate one, if nothing else for error
messages where we try to emit qualified names of columns.

But I don't see that the name needs to be anything we can refer to
elsewhere or anything faintly sane to type. Something like:
 "<pg anon subquery #[counter]>"

in line with our current generation of refcursor names.

We could reserve use of these names elsewhere, and probably should do
so to stop them leaking out if something manages to get hold of them
and re-use them via CREATE TABLE AS or something. But it seems
unlikely enough that I'm not too fussed if we don't explicitly reserve
them. Nobody is going to have a ref named "<pg anon subquery #1>"
already, I'm not concerned about BC if we do this.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Feb 24, 2017 at 1:04 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 23 February 2017 at 22:20, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> * Don't force/generate an alias at all.
>>
>>> I've no idea for this yet and Tom already was concerned what this might
>>> break. There are several places in the transform phase where the
>>> refnames are required (e.g. isLockedRefname()).
>>
>> Yeah.  This would be cleaner in some sense but also a lot more delicate.
>> Not sure it's worth the trouble.
>
> Personally I think we need to generate one, if nothing else for error
> messages where we try to emit qualified names of columns.
>
> But I don't see that the name needs to be anything we can refer to
> elsewhere or anything faintly sane to type. Something like:
>
>   "<pg anon subquery #[counter]>"
>
> in line with our current generation of refcursor names.

Isn't that a terribly unfriendly thing to include in an error message?I'd much rather see the column qualified with
whateverthe alias name
 
is inside the subquery than see it qualified with some internally
generated name that's completely meaningless.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Craig Ringer <craig@2ndquadrant.com> writes:
> Personally I think we need to generate one, if nothing else for error
> messages where we try to emit qualified names of columns.

Also for EXPLAIN, where there has to be a way to name everything.

> But I don't see that the name needs to be anything we can refer to
> elsewhere or anything faintly sane to type. Something like:
>   "<pg anon subquery #[counter]>"

-1.  "Make it ugly as sin and then pretend that nobody could conflict
with it" is neither formally correct nor nice to look at in the contexts
where people have to look at it.

I'm for something along the lines of "subquery_n" where we simply keep
incrementing n until we find a name that is not present in the query
already.  This is basically what ruleutils.c does now when it has to
cons up a unique table alias, which it must do in cases involving
inheritance.
        regards, tom lane

On Thu, Feb 23, 2017 at 01:27:29PM +0000, Greg Stark wrote:
> On 22 February 2017 at 15:08, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Indeed.  When I wrote the comment you're referring to, quite a few years
> > ago now, I thought that popular demand might force us to allow omitted
> > aliases.  But the demand never materialized.  At this point it seems
> > clear to me that there isn't really good reason to exceed the spec here.
> > It just encourages people to write unportable SQL code.
> 
> 
> Oh my. This bothers me all the time. I always assumed the reason it
> was like this was because the grammar would be ambiguous without it
> and it would require extreme measures to hack the grammar to work. If
> it's this easy I would totally be for it.
> 
> Offhand I think there are plenty of solutions for the problem of
> inventing names and I suspect any of them would work fine:
> 
> 1) Don't assign a name -- I would guess this would require some
> adjustments in the rule deparsing (i.e. views).
> 
> 2) Assign a name but add a flag indicating the name is autogenerated
> and shouldn't be used for resolving references and shouldn't be
> dumped. Then it shouldn't really matter if there's a conflict since
> the name is only used for things like error messages, not resolving
> references.
> 
> 3) thumb through all the names in the query and pick one that doesn't conflict.
> 
> For what it's worth while it wouldn't be a *bad* thing to avoid
> conflicts I think this is being held to an inconsistent standard here.
> It's not like there aren't similar situations elsewhere in the
> codebase where we just don't worry about this kind of thing:
> 
> => SELECT "?column"? FROM (select 1+1 as "?column?", 1+1) AS x;
> ERROR:  42703: column "?column" does not exist
> LINE 2: SELECT "?column"? FROM (select 1+1 as "?column?", 1+1) AS x;
>                ^
> HINT:  Perhaps you meant to reference the column "x.?column?" or the
> column "x.?column?".

That's because you transposed the two characters after column in your
target list:              XX
SELECT "?column"? FROM (select 1+1 as "?column?", 1+1) AS x;
SELECT "?column?" FROM (select 1+1 as "?column?", 1+1) AS x;

This is what you get when you do the second, which I'm assuming is
what you meant to do:

ERROR:  column reference "?column?" is ambiguous
LINE 1: SELECT "?column?" FROM (select 1+1 as "?column?", 1+1) AS x;

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On 2017-02-24 07:25, Robert Haas wrote:
> I don't think it's only Oracle that allows omitting the
> alias; I think there are a number of other systems that behave
> similarly.

SQLite, for example.

Making conversions from SQLite to Postgres easier is a Good Thing.
"subquery in FROM must have an alias" has caused me inconvenience doing
that as recently as last week.

-M-