Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-3084: Cache the sequence of table ref and materialized 
tuple ids during analysis.
......................................................................


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/2415/1/fe/src/main/java/com/cloudera/impala/analysis/TableRef.java
File fe/src/main/java/com/cloudera/impala/analysis/TableRef.java:

Line 111:   // Lists of table ref ids and materialized tuple ids of the full 
sequence of table
we talked about separating the tblref abstraction into a) a logical concept 
(table expr) and b) the physical underlying structure (BaseTblRef, etc.).

to which of those does the caching belong?


Line 365:   public List<TupleId> getAllTupleIds() {
if you want to make the distinction between tuple and tblref ids you should 
also update the function name.


Line 444:    * table ref ids and materialized tuple ids.
reference member names directly.


Line 449:     Preconditions.checkState(desc_ != null);
this depends on analyzeJoin() having been called for leftTblRef; checkState() 
for that.


Line 459:     if (leftTblRef_ != null) {
interleave so you only need one if-stmt.


http://gerrit.cloudera.org:8080/#/c/2415/1/fe/src/main/java/com/cloudera/impala/planner/SingleNodePlanner.java
File fe/src/main/java/com/cloudera/impala/planner/SingleNodePlanner.java:

Line 1389:         List<SlotId> lhsSlotIds = 
analyzer.getEquivSlots(slotDesc.getId(), lhsTblRefIds);
(rhsSid, ...


makes this a bit clearer


Line 1393:           // eliminate rows that would have been non-matches of an 
outer or anti join.
this seems to make assumptions about how the returned predicates are being used 
that aren't reflected in the function comment. update the comment.


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

Line 1540:    partitions=1/1 files=4 size=554.13MB
do you know what this is?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I298b8695c9f26644a395ca9f0e86040e3f5f3846
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-HasComments: Yes

Reply via email to