Alex Behm has posted comments on this change.

Change subject: IMPALA-3065/IMPALA-3062: Restrict !empty() predicates to scan 
nodes.
......................................................................


Patch Set 2:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/2399/2/fe/src/main/java/com/cloudera/impala/analysis/SelectStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/SelectStmt.java:

Line 176:     // superfluous predicates when analyzing a WITH-clause.
> why are they superfluous in a with-clause?
Expanded comment.


Line 301:    * - collection table ref is relative and non-correlated
> explain why it can't be correlated
Added a TODO that explains why.


Line 302:    * - collection table ref represents the rhs of an inner/cross or 
non-anti semi join
> remove "non-anti"
Done


http://gerrit.cloudera.org:8080/#/c/2399/2/testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/nested-collections.test:

Line 122: select b.item from functional.allcomplextypes a right semi join 
a.int_array_col b
> this is in effect an inner join.  recognizing that explicitly would probabl
Done


Line 297: # TODO: Transform the join op into an INNER JOIN.
> okay, we already have that todo.
Sure. Added a TODO in CollectionTableRef right before analyzeJoin(), and 
removed the TODOs in this file.


http://gerrit.cloudera.org:8080/#/c/2399/2/testdata/workloads/functional-query/queries/QueryTest/nested-types-runtime.test
File 
testdata/workloads/functional-query/queries/QueryTest/nested-types-runtime.test:

Line 427: select count(a.pos) from complextypestbl t1
> straight_join?
Good point. Done.


Line 432: 10
> only 10 rows?
Yes, complextypestbl is a small table and sufficient to cover one of the buggy 
cases.

Like Tim pointed out, covering the other case is trickier. I modified this test 
accordingly but kept the one below.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie975ce139a103285c4e9f93c59ce1f1d2aa71767
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.5.0_5.7.0
Gerrit-Owner: Alex Behm <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to