impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5211: Simplifying ifnull/isnull/nvl where conditional is a literal.
Date Fri, 25 Aug 2017 15:26:28 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-5211: Simplifying ifnull/isnull/nvl where conditional is a literal.
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7781/6/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java
File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java:

Line 105:    * Simplifies IFNULL by returning the corresponding child if the condition
by returning the appropriate child

(corresponding to what?)


http://gerrit.cloudera.org:8080/#/c/7781/6/testdata/workloads/functional-planner/queries/PlannerTest/simplify-expressions.test
File testdata/workloads/functional-planner/queries/PlannerTest/simplify-expressions.test:

Line 1: # ifnull simplified out because first argument is constant.
Remove.

Let's avoid planner tests as much as possible, esp. for things that are very unittestable
(like expr rewrites).
You can have the same test in ExprRewriterTest. Seeing the plan doesn't really give any more
coverage.


-- 
To view, visit http://gerrit.cloudera.org:8080/7781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9e4b8bf6fedd595f9ea54ffb30fc71a058c7f16c
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Philip Zeyliger <philip@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message