Marcel Kornacker has posted comments on this change. Change subject: IMPALA-2521: Add clustered hint to insert statements ......................................................................
Patch Set 8: (3 comments) i won't look at the substitution logic in detail because it gives me a headache. :) but please add that test. http://gerrit.cloudera.org:8080/#/c/4745/8/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java: Line 603: // Store exprs for Kudu key columns. add line breaks between if blocks, it's harder to navigate without those ('{' in vi let's you jump by a 'paragraph'). at least add one at l613 http://gerrit.cloudera.org:8080/#/c/4745/8/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java: Line 237: * the outputs of aggregation. Invoked for UnionStmt as well since something missing there. (i know, not your fault.) http://gerrit.cloudera.org:8080/#/c/4745/8/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test: Line 247: # IMPALA-2521: clustered insert into table adds sort node, correctly substituting exprs. you can't always see the correct substitution until you run the query (because the column labels can be the same even when you're pointing at the wrong tuple). let's add query tests as well, what you have so far should run (even if you won't see any benefit in the hdfs case). -- 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: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
