Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/8400 )
Change subject: IMPALA-4167: Support insert plan hints for CREATE TABLE AS SELECT ...................................................................... Patch Set 17: (3 comments) I have marked the lines where the rebase (patch set 16) led to non-trivial conflicts/broken tests. Patch set 17 contains some typo fixes + changes in kudu.test, which I forgot to run after rebase. http://gerrit.cloudera.org:8080/#/c/8400/17/fe/src/test/java/org/apache/impala/analysis/ParserTest.java File fe/src/test/java/org/apache/impala/analysis/ParserTest.java: http://gerrit.cloudera.org:8080/#/c/8400/17/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@303 PS17, Line 303: /** : * Creates an insert into, an insert overwrite, and a CTAS statement with : * the given hints and checks that the parsed hints are the same as the expected hints. : */ : private void TestInsertAndCtasHints(String insertPart, String ctasPart, : String[] hintStyle, String hints, String... expectedHints) { : String hintsPart = hintStyle[0] + hints + hintStyle[1]; : TestInsertStmtHints(String.format("insert %%s into %s %%s select * from t", : insertPart), hintsPart, expectedHints); : TestInsertStmtHints(String.format("insert %%s overwrite %s %%s select * from t", : insertPart), hintsPart, expectedHints); : TestCtasHints(String.format("create %s table %s as select * from t", : hintsPart, ctasPart), expectedHints); : } : : /** : * Injects hints into pattern and checks that the injected hints match the expected : * hints. This function covers both insert and upsert statements. : */ : private void TestInsertStmtHints(String pattern, String hint, String... expectedHints) { : for (InsertStmt.HintLocation loc: InsertStmt.HintLocation.values()) { : InsertStmt insertStmt = (InsertStmt) ParsesOk(InjectInsertHint(pattern, hint, loc)); : assertEquals(expectedHints, HintsToStrings(insertStmt.getPlanHints())); : } : } : : /** : * Injects hints into pattern and expect parser error on the injected hints. : * It covers both insert and upsert statements. : */ : private void ParserErrorOnInsertStmtHints(String pattern, String hint) { : for (InsertStmt.HintLocation loc: InsertStmt.HintLocation.values()) { : ParserError(InjectInsertHint(pattern, hint, loc)); : } : } IMPALA-4168: "Adds Oracle-style hint placement for INSERT/UPSERT" also contains similar changes to avoid test duplication ( https://gerrit.cloudera.org/#/c/8676/ ). http://gerrit.cloudera.org:8080/#/c/8400/17/testdata/workloads/functional-planner/queries/PlannerTest/ddl.test File testdata/workloads/functional-planner/queries/PlannerTest/ddl.test: http://gerrit.cloudera.org:8080/#/c/8400/17/testdata/workloads/functional-planner/queries/PlannerTest/ddl.test@210 PS17, Line 210: # noshuffle hint would lead to a different plan. : # Note that noclustered hint is added to ensure consistent plans on Impala 2.x and 3.x, : # because IMPALA-5293 changed clustered to be the default on 3.x. : create /*+ noclustered */table t partitioned by (year, month) as These tests were broken by IMPALA-5293: "Turn insert clustering on by default". IMPALA-5293 is not merged to Impala 2.x, while this change can be merged in my opinion. To avoid extra work, I have changed the tests to statements that should lead to the same plan on both branches. http://gerrit.cloudera.org:8080/#/c/8400/17/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test: http://gerrit.cloudera.org:8080/#/c/8400/17/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test@431 PS17, Line 431: primary key(id) partition by hash(id) partitions 3 stored as kudu as : select * from functional.alltypes; : ---- DISTRIBUTEDPLAN : INSERT INTO KUDU [default.t] : | : 01:EXCHANGE [KUDU(KuduPartition(functional.alltypes.id))] : | : 00:SCAN HDFS [functional.alltypes] : partitions=24/24 files=24 size=478.45KB : ==== : create /* +noclustered,noshuffle */ table t : primary key(id) partition by hash(id) partitions 3 stored as kudu as These tests broke because of IMPALA-6297: "Don't partition/sort for DML on unpartitioned Kudu table", -- To view, visit http://gerrit.cloudera.org:8080/8400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8d74bca999da8ae1bb89427c70841f33e3c56ab0 Gerrit-Change-Number: 8400 Gerrit-PatchSet: 17 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Comment-Date: Thu, 22 Feb 2018 15:11:33 +0000 Gerrit-HasComments: Yes
