impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5211: Simplifying conditionals (istrue, nullif, etc.)
Date Tue, 05 Sep 2017 21:47:41 GMT
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 <philip@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message