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 <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-HasComments: Yes

Reply via email to