Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9153 )
Change subject: IMPALA-5293: Turn insert clustering on by default ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/9153/3/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java: http://gerrit.cloudera.org:8080/#/c/9153/3/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@831 PS3, Line 831: analyzer.addWarning("Clustering is enabled by default, use 'noclustered' to " + I'm wondering if the warning is necessary. I don't think we generally warn in cases like this. I can imagine some users might add the hint to ensure the desired behaviour and they wouldn't want the warning in that case. http://gerrit.cloudera.org:8080/#/c/9153/3/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: http://gerrit.cloudera.org:8080/#/c/9153/3/fe/src/main/java/org/apache/impala/planner/Planner.java@615 PS3, Line 615: primay primary http://gerrit.cloudera.org:8080/#/c/9153/3/testdata/workloads/functional-planner/queries/PlannerTest/empty.test File testdata/workloads/functional-planner/queries/PlannerTest/empty.test: http://gerrit.cloudera.org:8080/#/c/9153/3/testdata/workloads/functional-planner/queries/PlannerTest/empty.test@302 PS3, Line 302: # TODO Alex: Is it a problem to have the SORT node inserted here? IMO this seems ok, the benefit of optimising this case would be very marginal. http://gerrit.cloudera.org:8080/#/c/9153/3/testdata/workloads/functional-planner/queries/PlannerTest/insert.test File testdata/workloads/functional-planner/queries/PlannerTest/insert.test: http://gerrit.cloudera.org:8080/#/c/9153/3/testdata/workloads/functional-planner/queries/PlannerTest/insert.test@101 PS3, Line 101: # IMPALA-5293: noclustered hint prevents adding sort node Do we also need to test the case when there's a noclustered hint and sort.columns, so that the hint is ignored? I couldn't see a test that checks we get the correct plan. http://gerrit.cloudera.org:8080/#/c/9153/3/testdata/workloads/functional-query/queries/QueryTest/insert.test File testdata/workloads/functional-query/queries/QueryTest/insert.test: http://gerrit.cloudera.org:8080/#/c/9153/3/testdata/workloads/functional-query/queries/QueryTest/insert.test@949 PS3, Line 949: # IMPALA-5293: noclustered with outer join, inline view, and TupleisNullPredicate It looks like this one should still be a clustered insert, since IMPALA-6280 was related specifically to clustered inserts. -- To view, visit http://gerrit.cloudera.org:8080/9153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idbf2368cf4415e6ecfa65058daf6ff87ef62f9d9 Gerrit-Change-Number: 9153 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 30 Jan 2018 18:35:43 +0000 Gerrit-HasComments: Yes
