Lars Volker has posted comments on this change.

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


Patch Set 4:

(8 comments)

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

Thanks for the comments. I had a look and it didn't add too much code, so I 
added kudu support and tests here. It also seemed to fit into the scope of the 
Jira.

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
Done


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...
Done


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


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?
Done


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


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


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, ...
Done


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


-- 
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