(intentional?) variable scope regression in ORDER BY

[cberner]

This query works in 0.160, but fails on master: SELECT SUM(x) x FROM (VALUES 1) t(x) ORDER BY SUM(x)

Due to:

Caused by: com.facebook.presto.sql.analyzer.SemanticException: Cannot nest aggregations inside aggregation 'sum': ["sum"("x")]
	at com.facebook.presto.sql.analyzer.AggregationAnalyzer$Visitor.visitFunctionCall(AggregationAnalyzer.java:294)
	at com.facebook.presto.sql.analyzer.AggregationAnalyzer$Visitor.visitFunctionCall(AggregationAnalyzer.java:151)
	at com.facebook.presto.sql.tree.FunctionCall.accept(FunctionCall.java:109)
	at com.facebook.presto.sql.tree.AstVisitor.process(AstVisitor.java:22)
	at com.facebook.presto.sql.analyzer.AggregationAnalyzer$Visitor.process(AggregationAnalyzer.java:520)
	at com.facebook.presto.sql.analyzer.AggregationAnalyzer.analyze(AggregationAnalyzer.java:146)
	at com.facebook.presto.sql.analyzer.StatementAnalyzer.verifyAggregations(StatementAnalyzer.java:1618)

This appears to be a regression caused by ec2e897. It's not clear to me if that commit intended to change the semantics of existing queries or not.

[sopel39]

This is intentional. x from ORDER BY references projection output: SUM(t.x). So ORDER BY expression is: SUM(SUM(t.x)), which is incorrect. This query should work when rewritten to:
1) SELECT SUM(x) x FROM (VALUES 1) t(x) ORDER BY x
2) SELECT SUM(x) x FROM (VALUES 1) t(x) ORDER BY SUM(t.x)

Please confirm: @martint @kokosing

[martint]

This is due to a fix that corrected a behavior that deviated from the SQL spec. See see #4698. The rules for variable resolution are now correct. Column references in the select clause are supposed to be resolved against names from the select clause.

We could probably improve the error message when aggregations in the order by clause reference an output column derived from another aggregation.

Note: although Presto supports aggregation functions in the order by clause, it's technically not valid SQL (according to ANSI spec).

[martint]

Since this potentially affects a lot of production queries at FB, we're going to introduce a temporary config flag to restore the old behavior while we work with users to update their queries. We plan to get rid of the flag and associated behavior early next year.