wangsheng has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17933 )

Change subject: IMPALA-10836: Add 'SimplifyCastExprRule' rule to rewrite cast 
expr in some situations
......................................................................


Patch Set 3:

(9 comments)

Thanks for carefully review, Quanlong!

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

http://gerrit.cloudera.org:8080/#/c/17933/2//COMMIT_MSG@7
PS2, Line 7: SimplifyCastExprRule
> Can we rename it to 'SimplifyCastExprRule'? We already have some similar na
Done


http://gerrit.cloudera.org:8080/#/c/17933/2//COMMIT_MSG@9
PS2, Line 9: add
> nit: adds
Done


http://gerrit.cloudera.org:8080/#/c/17933/2//COMMIT_MSG@11
PS2, Line 11: rules i
> nit: is the same
Done


http://gerrit.cloudera.org:8080/#/c/17933/2//COMMIT_MSG@12
PS2, Line 12: ing a n
> nit: is the same
Done


http://gerrit.cloudera.org:8080/#/c/17933/2//COMMIT_MSG@14
PS2, Line 14: move unnecessar
> nit: if any rules is matched
Done


http://gerrit.cloudera.org:8080/#/c/17933/2//COMMIT_MSG@15
PS2, Line 15:  and
> nit: casting
Done


http://gerrit.cloudera.org:8080/#/c/17933/2//COMMIT_MSG@16
PS2, Line 16: time and IO/memmory.
> nit: , especially when scanning lots of data.
Done


http://gerrit.cloudera.org:8080/#/c/17933/2/fe/src/main/java/org/apache/impala/rewrite/CastExprSimplifyRule.java
File fe/src/main/java/org/apache/impala/rewrite/CastExprSimplifyRule.java:

http://gerrit.cloudera.org:8080/#/c/17933/2/fe/src/main/java/org/apache/impala/rewrite/CastExprSimplifyRule.java@56
PS2, Line 56:
> Can we remove this check? So the rule can apply to more scenarios, e.g. CAS
Done


http://gerrit.cloudera.org:8080/#/c/17933/2/fe/src/main/java/org/apache/impala/rewrite/CastExprSimplifyRule.java@56
PS2, Line 56:
            :
            :
            :
            :
            :
            :
            :
            :
            :
> I think we can merge these two branches into one. We just check the first c
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8fac7100060d4e139a8b24d4795c6f279c55954
Gerrit-Change-Number: 17933
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Xianqing He <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: wangsheng <[email protected]>
Gerrit-Comment-Date: Mon, 25 Oct 2021 12:54:57 +0000
Gerrit-HasComments: Yes

Reply via email to