+Greg
On Wed, Jan 11, 2017 at 3:22 PM, Tim Armstrong (Code Review) <ger...@cloudera.org> wrote: > 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 <tmarsh...@cloudera.com> > Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> > Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> > Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> > Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> > Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> > Gerrit-HasComments: Yes
