Alex Behm has posted comments on this change. Change subject: IMPALA-5211: Simplifying conditionals (istrue, nullif, etc.) ......................................................................
Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/7829/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java: Line 160: if (expr.getChildren().size() == 1) { Sorry, but I think these simplifications are not needed because they are already addressed by FoldConstantsRule. Here, we only need to handle conditionals that can be simplified even when non-constant (constant exprs are handled in FoldConstantsRule). Line 220: * Simplifies nullif(a, b) when a and b are identical Why is equals() not sufficient on its own? Two identical literals should also return true for equals(). Seems much easier to deal with NullLiteral case specially in this rule, and otherwise only rely on equals(). Alternatively, we could follow the same approach we use for NVL2() - directly convert NULLIF() into an IF() in FunctionCallExpr. -- To view, visit http://gerrit.cloudera.org:8080/7829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id91ca968a0c0be44e1ec54ad8602f91a5cb2e0e5 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-HasComments: Yes
