impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-2521: Add clustered hint to insert statements
Date Wed, 19 Oct 2016 23:15:05 GMT
Alex Behm has posted comments on this change.

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

Patch Set 4:


Nice! Current PS looks good to me. Do you intent to add Kudu support in a separate patch?
File fe/src/main/java/org/apache/impala/analysis/

Line 255:       SlotRef origSlotRef = (SlotRef) smap.getLhs().get(i);
Seems clearer to call these inputSlotRef and outputSlotRef
File fe/src/main/java/org/apache/impala/analysis/

Line 149:    * output by the sort node. Done by materializing slot refs in the order-by and
... 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
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.
File fe/src/main/java/org/apache/impala/planner/

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

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

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I412153bd8435d792bd61dea268d7a3b884048f14
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-HasComments: Yes

View raw message