Обсуждение: Regression from 7.3 to 7.4

This testcase works in 7.3 but not in 7.4:

------------------------------------------

create table t1 (a int);
create table t2 (b int);
select * from t1, (select b as a from t2 group by a) as foo;
------------------------------------------

ERROR:  column "t2.b" must appear in the GROUP BY clause or be used in an 
aggregate function

I don't know if it's supposed to work to do a group by of an alias, but 
why not and it used to work.

--
/Dennis Björklund

Dennis Bjorklund <db@zigo.dhs.org> writes:
> This testcase works in 7.3 but not in 7.4:

> create table t1 (a int);
> create table t2 (b int);
> select * from t1, (select b as a from t2 group by a) as foo;

> ERROR:  column "t2.b" must appear in the GROUP BY clause or be used in an 
> aggregate function

> I don't know if it's supposed to work to do a group by of an alias, but 
> why not and it used to work.

This example strikes me as a good reason why we ought to deprecate and
eventually remove the capability for GROUP BY to reference output-list
aliases.  This is not legal per SQL spec, but we have accepted it for a
few versions now (I think since about 6.5).

The reason for the change in behavior is that 7.4 is trying to be
helpful for some related errors.  The subselect "foo" is not allowed to
reference columns of t1.  In 7.3 you'd get a plain "not found" error:

regression=# select * from t1, (select a from t2) as foo;
ERROR:  Attribute "a" not found

where now you get

regression=# select * from t1, (select a from t2) as foo;
ERROR:  subquery in FROM may not refer to other relations of same query level

The problem is that this check is made after parsing the subquery, so if
the subquery is internally inconsistent you'll get the subquery-local
error first.  In your example the subquery thinks it has a "GROUP BY
outer-reference" and so the check for ungrouped columns fails.

This change was made partly because it seemed to yield more useful error
messages, and partly in anticipation of supporting the SQL99 LATERAL
feature, in which such references *are* legal.  So while reverting the
change is one possible answer, it doesn't seem forward-looking.

Another tweak we could make is to cause findTargetlistEntry() to look
only for local variable names before looking for targetlist alias
matches.  This would effectively change the precedence for resolving
"GROUP BY x" to be (1) x as a local variable, (2) x as a targetlist
alias, (3) x as an outer variable; whereas the present search order is
(1), (3), (2).  AFAICS this does not break compatibility with either
SQL92 or SQL99 because both of them allow only case (1).  However this
could break existing queries that are relying on the non-aliased
behavior.  (Offhand I can't think of a really good reason to GROUP BY
an outer reference, but maybe there is one.)

Or we could bite the bullet and deprecate/remove the alias-reference
feature.  I think I was the one who put it in originally for GROUP BY,
but in hindsight it was a terrible idea --- it's caused way more
confusion than it's worth.

Comments?
        regards, tom lane

On Mon, 5 Apr 2004, Tom Lane wrote:

> This example strikes me as a good reason why we ought to deprecate and
> eventually remove the capability for GROUP BY to reference output-list
> aliases.  This is not legal per SQL spec,

Sticking to the SQL spec is (almost) always good.

> matches.  This would effectively change the precedence for resolving
> "GROUP BY x" to be (1) x as a local variable, (2) x as a targetlist
> alias, (3) x as an outer variable; whereas the present search order is
> (1), (3), (2).  AFAICS this does not break compatibility with either
> SQL92 or SQL99 because both of them allow only case (1).

This sounds to me as a usable solution that should be in forever or until
this pg extension is removed.

> However this could break existing queries that are relying on the
> non-aliased behavior.

This would be programs written for 7.4, In 7.3 and older the search order
was (1), (2) and there was no (3) if I understood you correctly? I'd 
rather stay compatible with older programs. I (as you) don't expect a lot 
of GROUP BY constant_value_from_outer_query.

> Or we could bite the bullet and deprecate/remove the alias-reference
> feature.  I think I was the one who put it in originally for GROUP BY,
> but in hindsight it was a terrible idea

Well, there are cases where it's nice to be able to do it. It's also 
something that a lot of people expect to work. I don't mind if you just 
implement what the standard says, but it's not my call to decide this.

-- 
/Dennis Björklund

Dennis Bjorklund <db@zigo.dhs.org> writes:
> On Mon, 5 Apr 2004, Tom Lane wrote:
>> However this could break existing queries that are relying on the
>> non-aliased behavior.

> This would be programs written for 7.4, In 7.3 and older the search order
> was (1), (2) and there was no (3) if I understood you correctly?

No, it's been (1), (3), (2) since 6.5 or before.
        regards, tom lane

I wrote:
> Dennis Bjorklund <db@zigo.dhs.org> writes:
>> This testcase works in 7.3 but not in 7.4:

>> create table t1 (a int);
>> create table t2 (b int);
>> select * from t1, (select b as a from t2 group by a) as foo;

> Another tweak we could make is to cause findTargetlistEntry() to look
> only for local variable names before looking for targetlist alias
> matches.  This would effectively change the precedence for resolving
> "GROUP BY x" to be (1) x as a local variable, (2) x as a targetlist
> alias, (3) x as an outer variable; whereas the present search order is
> (1), (3), (2).  AFAICS this does not break compatibility with either
> SQL92 or SQL99 because both of them allow only case (1).

I've patched both 7.4 and HEAD branches to do things this way.
        regards, tom lane