Tim Armstrong has posted comments on this change. Change subject: IMPALA-1861: Simplify conditionals with constant conditions ......................................................................
Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/5585/6/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java: Line 175: if (whenExpr instanceof BoolLiteral) { > Thanks Marcel for the explanation. I checked this in another DB engine, whi Given the complexity of some of the case/when clauses we see in practice I'm sure some or many users depend on the current evaluation order, particularly since this is the "obvious" behaviour. I think we should be careful doing this since my bet is that changing this will break someone's query that worked before. If we're going to change this it seems like at a minimum we should add a warning to the documentation about this and call it out clearly that this has changed in the commit message, release notes, etc (my personal preference is that we document the pre-existing behaviour and preserve it by only optimising this when all previous "WHEN" clauses were constant). -- 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: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
