Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11887 )
Change subject: IMPALA-7818: Standardize use of Expr predicates ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/Expr.java File fe/src/main/java/org/apache/impala/analysis/Expr.java: http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/Expr.java@1169 PS4, Line 1169: Expr. drop here, and other places where its not needed. http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java: http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@432 PS4, Line 432: Expr. can drop http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java File fe/src/main/java/org/apache/impala/analysis/LikePredicate.java: http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java@132 PS4, Line 132: IS_NON_NULL_LITERAL double-checking if this translation preserves the previous expression? isNullLiteral is replaced by the value variant but IS_NON_NULL_LITERAL uses the instanceof check. I can be convinced that it falls out to be the same, but it is not obvious. http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java File fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java: http://gerrit.cloudera.org:8080/#/c/11887/4/fe/src/main/java/org/apache/impala/rewrite/FoldConstantsRule.java@52 PS4, Line 52: IS_LITERAL_VALUE IS_LITERAL? -- To view, visit http://gerrit.cloudera.org:8080/11887 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I09a089c0f2484246e5c6444b78990fa9260c036a Gerrit-Change-Number: 11887 Gerrit-PatchSet: 4 Gerrit-Owner: Paul Rogers <par0...@yahoo.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Paul Rogers <par0...@yahoo.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Wed, 07 Nov 2018 19:32:21 +0000 Gerrit-HasComments: Yes