impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5211: Simplifying conditionals (istrue, nullif, etc.)
Date Wed, 06 Sep 2017 21:19:10 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-5211: Simplifying conditionals (istrue, nullif, etc.)

Patch Set 4:

File fe/src/main/java/org/apache/impala/rewrite/

Line 160:     if (expr.getChildren().size() == 1) {
> Yep, you're right. I'll remove this code, update comments to indicate that 
Before you add those existing tests to ExprRewriteRulesTest, please take a look at
to see whether there is already sufficient coverage there (my suspicion is yes, but better
to check). The entire is run with and without constant folding.

Line 220:    * Simplifies nullif(a, b) when a and b are identical
> I'm not sure what you mean by your suggestion " with NullLiteral cas
Sorry I got confused by the nullif() semantics, my suggestion with special casing the NullLiteral
does not make sense.

How about the following solution:
1. Convert nullif() to IF(x <=> y, x, NULL). Remove the BE implementation, leave the
tests. Note that IF(x=y, NULL, x) is not a correct transformation because NULL=NULL ->
2. Enhance the existing IF simplification to deal with the specific case above (you can use
expr.equals() just in the same way you would have here).

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Id91ca968a0c0be44e1ec54ad8602f91a5cb2e0e5
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Philip Zeyliger <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Philip Zeyliger <>
Gerrit-HasComments: Yes

View raw message