impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-2521: Add clustered hint to insert statements
Date Thu, 20 Oct 2016 04:42:12 GMT
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 <lv@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message