impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-1861: Simplify conditionals with constant conditions
Date Wed, 11 Jan 2017 23:22:13 GMT
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 <tmarshall@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message