impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marcel Kornacker <mar...@cloudera.com>
Subject Re: [Impala-ASF-CR] IMPALA-1861: Simplify conditionals with constant conditions
Date Wed, 11 Jan 2017 23:51:43 GMT
+Greg

On Wed, Jan 11, 2017 at 3:22 PM, Tim Armstrong (Code Review)
<gerrit@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 <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