Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/8660 )
Change subject: IMPALA-5929: Remove redundant explicit casts to string ...................................................................... Patch Set 4: (10 comments) http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java File fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java: http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java@31 PS4, Line 31: * "cast(<non-const expr> to <string type>) <eq/ne op> <string constant>" > string literal (not string constant) Done http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java@35 PS4, Line 35: * if the following is true: cast(cast(<string constant> as typeOf(<non-const > string literal (not string constant) Done http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java@36 PS4, Line 36: * expr>)) as typeOf(<original cast expr>)) == <string constant> > string literal (not string constant) Done http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java@39 PS4, Line 39: * <constant> is always on the right hand side and all constant Exprs have been > <string-literal> (not <constant>) Done http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java@48 PS4, Line 48: * Few cases that are are not rewritten as the redundancy test fails: > double "are" Done http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java@67 PS4, Line 67: !expr.getChild(0).isLiteral() && > not needed, we already require that child(0) is a CastExpr Done http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java@69 PS4, Line 69: (op == BinaryPredicate.Operator.EQ || op == BinaryPredicate.Operator.NE); > this is the cheapest check, might want to to the front of the conjunction o Done http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/main/java/org/apache/impala/rewrite/RemoveRedundantStringCast.java@75 PS4, Line 75: LiteralExpr constantExpr = (LiteralExpr) expr.getChild(1); > literalExpr Done http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java: http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@619 PS4, Line 619: // Works for other string types > // Works for VARCHAR/CHAR Done http://gerrit.cloudera.org:8080/#/c/8660/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java@641 PS4, Line 641: > remove extra blank line Done -- To view, visit http://gerrit.cloudera.org:8080/8660 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91b7c6452d0693115f9b9ed9ba09f3ffe0f36b2b Gerrit-Change-Number: 8660 Gerrit-PatchSet: 4 Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 19 Dec 2017 19:01:10 +0000 Gerrit-HasComments: Yes
