Aman Sinha has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16353 )

Change subject: IMPALA-10096: Use the original ordinal if the group by ordinal 
reference is a constant int
......................................................................


Patch Set 2:

(6 comments)

A few grammatical and couple of other comments. Code changes LGTM.

http://gerrit.cloudera.org:8080/#/c/16353/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16353/2//COMMIT_MSG@9
PS2, Line 9: using the
nit: 'uses' instead of 'using'


http://gerrit.cloudera.org:8080/#/c/16353/2//COMMIT_MSG@11
PS2, Line 11: throw
nit: 'throw an'


http://gerrit.cloudera.org:8080/#/c/16353/2//COMMIT_MSG@12
PS2, Line 12: sql
strictly, it is not the sql string but the expression; you could rephrase as 
'after the expression has been rewritten'.


http://gerrit.cloudera.org:8080/#/c/16353/2//COMMIT_MSG@19
PS2, Line 19: there are
remove 'there are',  the sentence '..since no rewriting happens' is complete.


http://gerrit.cloudera.org:8080/#/c/16353/2//COMMIT_MSG@22
PS2, Line 22:  - Added new unit tests with ordinal in SELECT and GROUP BY
nit:  ordinal is only in the GROUP BY whereas SELECT has the INT literal.


http://gerrit.cloudera.org:8080/#/c/16353/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:

http://gerrit.cloudera.org:8080/#/c/16353/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@759
PS2, Line 759:       // use the original ordinal if the analyzed expr is a 
constant int
maybe worth stating that this applies to all INT_LITERALs which covers BIGINT 
also.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I34f659d15073d69aa0a4685f56ad94557df86560
Gerrit-Change-Number: 16353
Gerrit-PatchSet: 2
Gerrit-Owner: Xianqing He <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Xianqing He <[email protected]>
Gerrit-Comment-Date: Sat, 05 Sep 2020 03:08:50 +0000
Gerrit-HasComments: Yes

Reply via email to