Alex Behm has posted comments on this change.

Change subject: IMPALA-2521: Add clustered hint to insert statements
......................................................................


Patch Set 4:

(8 comments)

Nice! Current PS looks good to me. Do you intent to add Kudu support in a 
separate patch?

http://gerrit.cloudera.org:8080/#/c/4745/4/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java:

Line 255:       SlotRef origSlotRef = (SlotRef) smap.getLhs().get(i);
Seems clearer to call these inputSlotRef and outputSlotRef


http://gerrit.cloudera.org:8080/#/c/4745/4/fe/src/main/java/org/apache/impala/analysis/SortInfo.java
File fe/src/main/java/org/apache/impala/analysis/SortInfo.java:

Line 149:    * output by the sort node. Done by materializing slot refs in the 
order-by and result
... and given result expressions...


Line 151:    * slot refs into the new tuple. This simplifies sorting logic for 
total and top-n
the sorting logic


Line 164:     // The tuple descriptor for the sort output. It will contain the 
materialized tuples,
Not clear what "It" refers to in the second sentence, can you rephrase?


Line 185:     // ones that point to the slot refs the new, materialized input 
rows.
.. slot refs into the sort's output tuple.


Need to be careful with the terms 'tuple' and 'row' because they have a very 
specific and different meanings in Impala.


Line 188:     // Update the tuple descriptor used to materialize the input 
tuple of the sort.
... used to materialize the input of the sort.


http://gerrit.cloudera.org:8080/#/c/4745/4/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:

Line 496:    * Insert a sort node into the plan, depending on the 
clustered/noclustered plan hint.
Insert a sort node on top of the plan, ...


Line 505:     List<Expr> partitionExprs = 
Lists.newArrayList(insertStmt.getPartitionKeyExprs());
orderingExprs


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I412153bd8435d792bd61dea268d7a3b884048f14
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to