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 <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Wed, 15 Dec 2021 14:53:10 +0000 Gerrit-HasComments: Yes
