Alex Behm has posted comments on this change. Change subject: PREVIEW IMPALA-2521: Add clustered hint to insert statements ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/4745/1/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java: Line 269: * clustered/noclustered plan hint. The clustering re-order the data in 'inputFragment' Better to be specific: Say that it adds a sort node that orders on the partition columns of the target table. Line 272: // TODO: this function shares some code with QueryStmt::createSortTupleInfo(). However You could move most of the code into SortInfo.createSortTupleInfo(List<Expr> resultExprs); Line 309: sortTupleDesc.setIsMaterialized(true); To indicate that this tuple is a physical tuple needed during runtime execution and not a "virtual" tuple like e.g. the tuple of an inline view (which never gets materialized). Line 324: // TODO Why is this needed here but not in QueryStmt.java? If it is omitted, the query Before plan generation we call QueryStmt.materializeRequiredSlots() to make sure all slots needed to evaluate that QueryStmt are marked as materialized. However, we have no such mechanism here because we are adding a plan node that does not come from a SQL clause, so we must mark the slots as materialized manually. http://gerrit.cloudera.org:8080/#/c/4745/1/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: Line 144: rootFragment = distributedPlanner.addClusteringFragment( 1. we should also do this for single-node plans, so adding the clustering doesn't really belong in the DistributedPlanner 2. we are not adding a new fragment, so I'd rephrase this addClusteringNode() or similar -- 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: 1 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
