Marcel Kornacker has posted comments on this change. Change subject: IMPALA-1861: Simplify conditionals with constant conditions ......................................................................
Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/5585/4/fe/src/main/java/org/apache/impala/rewrite/NormalizeExprsRule.java File fe/src/main/java/org/apache/impala/rewrite/NormalizeExprsRule.java: Line 50: if (!(expr.getChild(0) instanceof BoolLiteral) why is isLiteral() not enough? (what other literal could it be?) http://gerrit.cloudera.org:8080/#/c/5585/3/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java: Line 65: /** > There will be a number of other functions in the future, eg. 'ifnull', 'isf then spell this out in a todo Line 132: * returned. > It occurs to me - we tried to do lazy creation here to minimize object crea i don't think it'll have any measurable impact on runtime, and the code won't get much simpler (plus you need some additional code for the check), so maybe not. but try it out if you want and see what comes out. http://gerrit.cloudera.org:8080/#/c/5585/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java: Line 91: * NormalizeExprsRule ensures will be the left child, according to the following rules: spell out rule dependencies in the class comment (and also in analysisctx where you put the rules together in a particular order, otherwise it's easy to break inadvertently) Line 143: // True if this CASE has a FALSE or NULL condition that can be simplified. 'simplified' -> 'removed' (even more specific) Line 148: Expr child = expr.getChild(i); why don't you do if (child instanceof nullliteral) { foundFalseExpr = true; continue; } here and then you don't have to deal with it anymore, such as in l175 -- To view, visit http://gerrit.cloudera.org:8080/5585 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id70aaf9fd99f64bd98175b7e2dbba28f350e7d3b Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
