Alex Behm has posted comments on this change.

Change subject: IMPALA-5016: Try to rewrite coalesce() function in 
SimplifyConditionalsRule.
......................................................................


Patch Set 3:

(4 comments)

Last round of minor comments. This patch looks good. Nice work!

http://gerrit.cloudera.org:8080/#/c/7015/3/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java:

Line 105:    * COALESCE(<partition-slotref>, a, b) -> <partition-slotref>, when 
the partition column does not contain NULL.
long line


Line 128:    * Check child expr is nullable, assume child has been rewritten by 
constants folding rule.
Checks if the given child expr is nullable.

You can remove the part about constant folding, this function does what it's 
supposed to regardless of whether folding has been applied or not.


http://gerrit.cloudera.org:8080/#/c/7015/3/fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriteRulesTest.java:

Line 58:   public Expr RewritesOk(String tableName, String expr, 
List<ExprRewriteRule> rules, String expectedExpr)
long line


Line 376:     RewritesOk("functional_kudu.alltypessmall", "coalesce(cast(id as 
string), string_col)", rule, "CAST(id AS STRING)");
long line


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0325a9e437261b15313bbdf2f22d83376a84b575
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: yu feng <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-HasComments: Yes

Reply via email to