impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] PREVIEW IMPALA-2521: Add clustered hint to insert statements
Date Tue, 18 Oct 2016 05:49:19 GMT
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 <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