Alex Behm has posted comments on this change. Change subject: IMPALA-2521: Add clustered hint to insert statements ......................................................................
Patch Set 8: Code-Review+1 (3 comments) I'm happy with the patch after the remaining comments. I imagine Marcel will want to take another pass, so only giving +1. http://gerrit.cloudera.org:8080/#/c/4745/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java: Line 602: } > I added a test for this, however it also seems to work without the substitu Thanks. The reason why it works without is that SortNode.init() during planning will substitute the ordering exprs appropriately. However, it's cleaner to leave your changes because the PK exprs do need to be substituted like the other ones, but it just happens to work in this case because of how the PK exprs are used exactly. Line 604: if (matchFound && table_ instanceof KuduTable) { > Done, though after finding that List supports contains I might as well inli I vote for keep 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 249: select id, bool_col, tinyint_col, smallint_col, int_col, bigint_col, float_col, double_col, use fewer columns, add an analytic function in the inline view -- 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
