Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18099 )

Change subject: IMPALA-11049: Substitute order by elements when creating 
SortInfo
......................................................................


Patch Set 1:

(6 comments)

Thanks for working on this. I've only spotted a few nits.

http://gerrit.cloudera.org:8080/#/c/18099/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18099/1//COMMIT_MSG@12
PS1, Line 12: will caused
nit: causes


http://gerrit.cloudera.org:8080/#/c/18099/1//COMMIT_MSG@13
PS1, Line 13: 'INVALID' type
Maybe it's worth to mention that they are INVALID type because they weren't not 
analyzed.


http://gerrit.cloudera.org:8080/#/c/18099/1//COMMIT_MSG@15
PS1, Line 15: But Pay attention,
            : this will changed explain result when printing sql.
I think we are already printing out the rewritten query in explain. E.g. for a 
table with 3 integer columns:

 explain select cast(i as INT), cast(j as INT), max(k) from ice_void group by 
2, 1;

In EXPLAIN output:

  Analyzed query: SELECT i, j, max(k) FROM `default`.ice_void GROUP BY j, i

Or if I'm missing something, then could you provide an example in the commit 
message?


http://gerrit.cloudera.org:8080/#/c/18099/1//COMMIT_MSG@16
PS1, Line 16: changed
nit: change


http://gerrit.cloudera.org:8080/#/c/18099/1//COMMIT_MSG@17
PS1, Line 17: to
            : keep exprs must analyzed before rewrite.
nit: to ensure that exprs are already analyzed.


http://gerrit.cloudera.org:8080/#/c/18099/1/fe/src/main/java/org/apache/impala/rewrite/SimplifyCastExprRule.java
File fe/src/main/java/org/apache/impala/rewrite/SimplifyCastExprRule.java:

http://gerrit.cloudera.org:8080/#/c/18099/1/fe/src/main/java/org/apache/impala/rewrite/SimplifyCastExprRule.java@61
PS1, Line 61: Expr must execute analyze before rewrite
'castExpr' must be analyzed before rewrite. Maybe we could add this as an error 
message instead of code comment.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2780e04a6d5a32e224cd0470cf6f166a832363ec
Gerrit-Change-Number: 18099
Gerrit-PatchSet: 1
Gerrit-Owner: wangsheng <sky...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Reviewer: wangsheng <sky...@163.com>
Gerrit-Comment-Date: Wed, 15 Dec 2021 14:53:10 +0000
Gerrit-HasComments: Yes

Reply via email to