Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8801 )

Change subject: IMPALA-5191: Standardize column alias behavior
......................................................................


Patch Set 7:

(4 comments)

Sorry for the delay! I had missed your update. I did not want to stall progress 
on this.

http://gerrit.cloudera.org:8080/#/c/8801/9/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java:

http://gerrit.cloudera.org:8080/#/c/8801/9/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@274
PS9, Line 274:   protected Expr getFirstAmbiguousAlias(List<Expr> exprs) {
Simpler: We should substitute by ordinal or alias but not both to avoid an 
incorrect double substitution.


http://gerrit.cloudera.org:8080/#/c/8801/9/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@278
PS9, Line 278:     return null;
Integrate this into L271, i.e.:

An ordinal is an integer NumericLiteral that refers to a select-list expression 
by ordinal. An alias is a SlotRef that matches the alias of a select-list 
expression (tracked by 'aliasMap_').


http://gerrit.cloudera.org:8080/#/c/8801/9/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@304
PS9, Line 304:           expr.analyze(analyzer);
Substitutes top-level ordinals and aliases. Does not substitute ordinals and 
aliases in subexpressions.


http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:

http://gerrit.cloudera.org:8080/#/c/8801/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@985
PS7, Line 985:     // IMPALA-5191: In GROUP BY, HAVING, ORDER BY, aliases and 
ordinals must only be
> That's fine, maybe it's not worth consolidating the tests. I'll think more
Did you address this issue with constant folding and ordinals?



--
To view, visit http://gerrit.cloudera.org:8080/8801
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0f82483b486acf6953876cfa672b0d034f3709a8
Gerrit-Change-Number: 8801
Gerrit-PatchSet: 7
Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jan 2018 23:46:34 +0000
Gerrit-HasComments: Yes

Reply via email to