You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/02/23 22:51:36 UTC
[09/15] impala git commit: IMPALA-4167: Support insert plan hints for
CREATE TABLE AS SELECT
IMPALA-4167: Support insert plan hints for CREATE TABLE AS SELECT
This change adds support for "clustered", "noclustered", "shuffle" and
"noshuffle" hints in CTAS statement.
Example:
create /*+ clustered,noshuffle */ table t partitioned by (year, month)
as select * from functional.alltypes
The effect of these hints are the same as in insert statements:
clustered:
Sort locally by partition expression before insert to ensure that only
one partition is written at a time. The goal is to reduce the number of
files kept open / buffers kept in memory simultaneously.
noclustered:
Do not sort by primary key before insert to Kudu table. No effect on HDFS
tables currently, as this is their default behavior.
shuffle:
Forces the planner to add an exchange node that repartitions by the
partition expression of the output table. This means that a partition
will be written only by a single node, which minimizes the global
number of simultaneous writes.
If only one partition is written (because all partitioning columns
are constant or the target table is not partitioned), then the shuffle
hint leads to a plan where all rows are merged at the coordinator where
the table sink is executed.
noshuffle:
Do not add exchange node before insert to partitioned tables.
The parser needed some modifications to be able to differentiate between
CREATE statements that allow hints (CTAS), and CREATE statements that
do not (every other type of CREATE statements). As a result, KW_CREATE
was moved from tbl_def_without_col_defs to statement rules.
Testing:
The parser tests mirror the tests of INSERT, while analysis and planner
tests are minimal, as the underlying logic is the same as for INSERT.
Query tests are not created, as the hints have no effect on
the DDL part of CTAS, and the actual query ran is the same as in
the insert case.
Change-Id: I8d74bca999da8ae1bb89427c70841f33e3c56ab0
Reviewed-on: http://gerrit.cloudera.org:8080/8400
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
Reviewed-on: http://gerrit.cloudera.org:8080/9422
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Tim Armstrong <ta...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/07d9f71a
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/07d9f71a
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/07d9f71a
Branch: refs/heads/2.x
Commit: 07d9f71a9359d0b4b668088678bbbfb59dff9142
Parents: 31d197a
Author: Csaba Ringhofer <cs...@cloudera.com>
Authored: Thu Oct 26 22:15:51 2017 +0200
Committer: Tim Armstrong <ta...@cloudera.com>
Committed: Fri Feb 23 22:51:01 2018 +0000
----------------------------------------------------------------------
fe/src/main/cup/sql-parser.cup | 54 ++++++++----
.../analysis/CreateTableAsSelectStmt.java | 37 ++++++---
.../apache/impala/analysis/AnalyzeDDLTest.java | 28 +++++++
.../impala/analysis/AnalyzeStmtsTest.java | 7 +-
.../org/apache/impala/analysis/ParserTest.java | 86 +++++++++++++-------
.../org/apache/impala/analysis/ToSqlTest.java | 7 +-
.../apache/impala/common/FrontendTestBase.java | 5 ++
.../queries/PlannerTest/ddl.test | 31 +++++++
.../queries/PlannerTest/kudu.test | 21 +++++
9 files changed, 211 insertions(+), 65 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/07d9f71a/fe/src/main/cup/sql-parser.cup
----------------------------------------------------------------------
diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup
index 38183f8..9f55dda 100644
--- a/fe/src/main/cup/sql-parser.cup
+++ b/fe/src/main/cup/sql-parser.cup
@@ -383,7 +383,7 @@ nonterminal Subquery subquery;
nonterminal JoinOperator join_operator;
nonterminal opt_inner, opt_outer;
nonterminal PlanHint plan_hint;
-nonterminal List<PlanHint> opt_plan_hints, plan_hint_list;
+nonterminal List<PlanHint> plan_hints, opt_plan_hints, plan_hint_list;
nonterminal TypeDef type_def;
nonterminal Type type;
nonterminal Expr sign_chain_expr;
@@ -417,6 +417,7 @@ nonterminal DropStatsStmt drop_stats_stmt;
nonterminal DropTableOrViewStmt drop_tbl_or_view_stmt;
nonterminal CreateDbStmt create_db_stmt;
nonterminal CreateTableAsSelectStmt create_tbl_as_select_stmt;
+nonterminal CreateTableAsSelectStmt.CtasParams create_tbl_as_select_params;
nonterminal CreateTableLikeStmt create_tbl_like_stmt;
nonterminal CreateTableStmt create_tbl_stmt;
nonterminal TableDef tbl_def_without_col_defs, tbl_def_with_col_defs;
@@ -1103,13 +1104,30 @@ create_db_stmt ::=
{: RESULT = new CreateDbStmt(db_name, comment, location, if_not_exists); :}
;
+
+// Merging the two cases using opt_plan_hints would lead to reduce-reduce conflict,
+// because if there are no hints, CTAS cannot be distinguished from normal CREATE
+// statements until the AS SELECT part, but the decision whether to reduce empty string
+// to opt_plan_hints must happen before reaching that part.
create_tbl_as_select_stmt ::=
+ KW_CREATE create_tbl_as_select_params:ctas_params
+ {:
+ RESULT = new CreateTableAsSelectStmt(ctas_params, null);
+ :}
+ | KW_CREATE plan_hints:hints create_tbl_as_select_params:ctas_params
+ {:
+ RESULT = new CreateTableAsSelectStmt(ctas_params, hints);
+ :}
+ ;
+
+create_tbl_as_select_params ::=
tbl_def_without_col_defs:tbl_def
tbl_options:options
KW_AS query_stmt:select_stmt
{:
tbl_def.setOptions(options);
- RESULT = new CreateTableAsSelectStmt(new CreateTableStmt(tbl_def), select_stmt, null);
+ RESULT = new CreateTableAsSelectStmt.CtasParams(new CreateTableStmt(tbl_def),
+ select_stmt, null);
:}
| tbl_def_without_col_defs:tbl_def
// An optional clause cannot be used directly below because it would conflict with
@@ -1122,7 +1140,8 @@ create_tbl_as_select_stmt ::=
tbl_def.getPrimaryKeyColumnNames().addAll(primary_keys);
tbl_def.getKuduPartitionParams().addAll(partition_params.getKuduPartitionParams());
tbl_def.setOptions(options);
- RESULT = new CreateTableAsSelectStmt(new CreateTableStmt(tbl_def), select_stmt, null);
+ RESULT = new CreateTableAsSelectStmt.CtasParams(new CreateTableStmt(tbl_def),
+ select_stmt, null);
:}
| tbl_def_without_col_defs:tbl_def
KW_PARTITIONED KW_BY LPAREN ident_list:partition_cols RPAREN
@@ -1130,19 +1149,19 @@ create_tbl_as_select_stmt ::=
KW_AS query_stmt:select_stmt
{:
tbl_def.setOptions(options);
- RESULT = new CreateTableAsSelectStmt(new CreateTableStmt(tbl_def),
+ RESULT = new CreateTableAsSelectStmt.CtasParams(new CreateTableStmt(tbl_def),
select_stmt, partition_cols);
:}
;
create_tbl_stmt ::=
- tbl_def_without_col_defs:tbl_def
+ KW_CREATE tbl_def_without_col_defs:tbl_def
tbl_options:options
{:
tbl_def.setOptions(options);
RESULT = new CreateTableStmt(tbl_def);
:}
- | tbl_def_without_col_defs:tbl_def
+ | KW_CREATE tbl_def_without_col_defs:tbl_def
// If "opt_tbl_data_layout" were used instead so that this rule could be combined with
// the rule above, there would be a conflict with the first rule in
// "create_tbl_as_select_stmt".
@@ -1154,7 +1173,7 @@ create_tbl_stmt ::=
create_tbl_stmt.getPartitionColumnDefs().addAll(partition_column_defs);
RESULT = create_tbl_stmt;
:}
- | tbl_def_with_col_defs:tbl_def
+ | KW_CREATE tbl_def_with_col_defs:tbl_def
opt_tbl_data_layout:data_layout
tbl_options:options
{:
@@ -1163,7 +1182,7 @@ create_tbl_stmt ::=
tbl_def.setOptions(options);
RESULT = new CreateTableStmt(tbl_def);
:}
- | tbl_def_with_col_defs:tbl_def
+ | KW_CREATE tbl_def_with_col_defs:tbl_def
KW_PRODUCED KW_BY KW_DATA source_ident:is_source_id ident_or_default:data_src_name
opt_init_string_val:init_string
opt_comment_val:comment
@@ -1177,7 +1196,7 @@ create_tbl_stmt ::=
RESULT = new CreateTableDataSrcStmt(new CreateTableStmt(tbl_def),
data_src_name, init_string);
:}
- | tbl_def_without_col_defs:tbl_def
+ | KW_CREATE tbl_def_without_col_defs:tbl_def
KW_LIKE file_format_val:schema_file_format
STRING_LITERAL:schema_location
opt_tbl_data_layout:data_layout
@@ -1197,7 +1216,7 @@ create_tbl_stmt ::=
// TODO: Refactor the CREATE TABLE statements to improve the grammar and the way we
// handle table options.
create_tbl_like_stmt ::=
- tbl_def_without_col_defs:tbl_def
+ KW_CREATE tbl_def_without_col_defs:tbl_def
KW_LIKE table_name:other_table
opt_comment_val:comment
file_format_create_table_val:file_format location_val:location
@@ -1207,7 +1226,7 @@ create_tbl_like_stmt ::=
:}
// This extra production is necessary since without it the parser will not be able to
// parse "CREATE TABLE A LIKE B".
- | tbl_def_without_col_defs:tbl_def
+ | KW_CREATE tbl_def_without_col_defs:tbl_def
opt_sort_cols:sort_cols
KW_LIKE table_name:other_table
opt_comment_val:comment
@@ -1221,8 +1240,8 @@ create_tbl_like_stmt ::=
// Used for creating tables where the schema is inferred externally, e.g., from an Avro
// schema, Kudu table or query statement.
tbl_def_without_col_defs ::=
- KW_CREATE external_val:external KW_TABLE if_not_exists_val:if_not_exists
- table_name:table
+ external_val:external KW_TABLE
+ if_not_exists_val:if_not_exists table_name:table
{: RESULT = new TableDef(table, external, if_not_exists); :}
;
@@ -2529,6 +2548,13 @@ opt_outer ::=
;
opt_plan_hints ::=
+ plan_hints:hints
+ {: RESULT = hints; :}
+ | /* empty */
+ {: RESULT = Lists.newArrayList(); :}
+ ;
+
+plan_hints ::=
COMMENTED_PLAN_HINT_START plan_hint_list:hints COMMENTED_PLAN_HINT_END
{: RESULT = hints; :}
/* legacy straight_join hint style */
@@ -2537,8 +2563,6 @@ opt_plan_hints ::=
/* legacy plan-hint style */
| LBRACKET plan_hint_list:hints RBRACKET
{: RESULT = hints; :}
- | /* empty */
- {: RESULT = Lists.newArrayList(); :}
;
plan_hint ::=
http://git-wip-us.apache.org/repos/asf/impala/blob/07d9f71a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
index c5c092d..222b99a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
@@ -66,23 +66,40 @@ public class CreateTableAsSelectStmt extends StatementBase {
EnumSet.of(THdfsFileFormat.PARQUET, THdfsFileFormat.TEXT, THdfsFileFormat.KUDU);
/**
+ * Helper class for parsing.
+ * Contains every parameter of the constructor with the exception of hints. This is
+ * needed to keep the production rules that check for optional hints separate from the
+ * rules that check for optional partition info. Merging these independent rules would
+ * make it necessary to create rules for every combination of them.
+ */
+ public static class CtasParams {
+ public CreateTableStmt createStmt;
+ public QueryStmt queryStmt;
+ public List<String> partitionKeys;
+
+ public CtasParams(CreateTableStmt createStmt, QueryStmt queryStmt,
+ List<String> partitionKeys) {
+ this.createStmt = Preconditions.checkNotNull(createStmt);
+ this.queryStmt = Preconditions.checkNotNull(queryStmt);
+ this.partitionKeys = partitionKeys;
+ }
+ }
+
+ /**
* Builds a CREATE TABLE AS SELECT statement
*/
- public CreateTableAsSelectStmt(CreateTableStmt createStmt, QueryStmt queryStmt,
- List<String> partitionKeys) {
- Preconditions.checkNotNull(queryStmt);
- Preconditions.checkNotNull(createStmt);
- createStmt_ = createStmt;
- partitionKeys_ = partitionKeys;
+ public CreateTableAsSelectStmt(CtasParams params, List<PlanHint> planHints) {
+ createStmt_ = params.createStmt;
+ partitionKeys_ = params.partitionKeys;
List<PartitionKeyValue> pkvs = null;
- if (partitionKeys != null) {
+ if (partitionKeys_ != null) {
pkvs = Lists.newArrayList();
- for (String key: partitionKeys) {
+ for (String key: partitionKeys_) {
pkvs.add(new PartitionKeyValue(key, null));
}
}
- insertStmt_ = InsertStmt.createInsert(
- null, createStmt.getTblName(), false, pkvs, null, null, queryStmt, null);
+ insertStmt_ = InsertStmt.createInsert(null, createStmt_.getTblName(), false, pkvs,
+ planHints, null, params.queryStmt, null);
}
public QueryStmt getQueryStmt() { return insertStmt_.getQueryStmt(); }
http://git-wip-us.apache.org/repos/asf/impala/blob/07d9f71a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
index ee4ed0e..56f81e3 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
@@ -1830,6 +1830,34 @@ public class AnalyzeDDLTest extends FrontendTestBase {
}
@Test
+ public void TestCreateTableAsSelectWithHints() throws AnalysisException {
+ // Test if CTAS hints are analyzed correctly and that conflicting hints
+ // result in error.
+ // The tests here are minimal, because other tests already cover this logic:
+ // - ParserTests#TestPlanHints tests if hints are set correctly during parsing.
+ // - AnalyzeStmtsTest#TestInsertHints tests the analyzes of insert hints, which
+ // is the same as the analyzes of CTAS hints.
+ for (String[] hintStyle: hintStyles_) {
+ String prefix = hintStyle[0];
+ String suffix = hintStyle[1];
+ // Test plan hints for partitioned Hdfs tables.
+ AnalyzesOk(String.format("create %sshuffle%s table t " +
+ "partitioned by (year, month) as select * from functional.alltypes",
+ prefix, suffix));
+ // Warn on unrecognized hints.
+ AnalyzesOk(String.format("create %sbadhint%s table t " +
+ "partitioned by (year, month) as select * from functional.alltypes",
+ prefix, suffix),
+ "INSERT hint not recognized: badhint");
+ // Conflicting plan hints.
+ AnalysisError(String.format("create %sshuffle,noshuffle%s table t " +
+ "partitioned by (year, month) as " +
+ "select * from functional.alltypes", prefix, suffix),
+ "Conflicting INSERT hints: shuffle and noshuffle");
+ }
+ }
+
+ @Test
public void TestCreateTableLike() throws AnalysisException {
AnalyzesOk("create table if not exists functional.new_tbl like functional.alltypes");
AnalyzesOk("create table functional.like_view like functional.view_view");
http://git-wip-us.apache.org/repos/asf/impala/blob/07d9f71a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
index cdcb9bc..ab66482 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
@@ -1737,12 +1737,7 @@ public class AnalyzeStmtsTest extends AnalyzerTest {
@Test
public void TestSelectListHints() throws AnalysisException {
- String[][] hintStyles = new String[][] {
- new String[] { "/* +", "*/" }, // traditional commented hint
- new String[] { "\n-- +", "\n" }, // eol commented hint
- new String[] { "", "" } // without surrounding characters
- };
- for (String[] hintStyle: hintStyles) {
+ for (String[] hintStyle: hintStyles_) {
String prefix = hintStyle[0];
String suffix = hintStyle[1];
AnalyzesOk(String.format(
http://git-wip-us.apache.org/repos/asf/impala/blob/07d9f71a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
index 8dd4898..178539f 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
@@ -300,11 +300,19 @@ public class ParserTest extends FrontendTestBase {
assertEquals(Lists.newArrayList(expectedHints), actualHints);
}
- private void VerifyHints(List<PlanHint> actualHints, String... expectedHints) {
- List<String> stringHints = Lists.newArrayList();
- for (PlanHint hint: actualHints) stringHints.add(hint.toString());
- if (stringHints.isEmpty()) stringHints = Lists.newArrayList((String) null);
- assertEquals(Lists.newArrayList(expectedHints), stringHints);
+ /**
+ * 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);
}
/**
@@ -313,8 +321,8 @@ public class ParserTest extends FrontendTestBase {
*/
private void TestInsertStmtHints(String pattern, String hint, String... expectedHints) {
for (InsertStmt.HintLocation loc: InsertStmt.HintLocation.values()) {
- VerifyHints(((InsertStmt) ParsesOk(InjectInsertHint(pattern, hint, loc)))
- .getPlanHints(), expectedHints);
+ InsertStmt insertStmt = (InsertStmt) ParsesOk(InjectInsertHint(pattern, hint, loc));
+ assertEquals(expectedHints, HintsToStrings(insertStmt.getPlanHints()));
}
}
@@ -328,20 +336,28 @@ public class ParserTest extends FrontendTestBase {
}
}
+ /**
+ * Parses stmt and checks that the CTAS hints stmt are the expected hints.
+ */
+ private void TestCtasHints(String stmt, String... expectedHints) {
+ CreateTableAsSelectStmt ctasStmt = (CreateTableAsSelectStmt) ParsesOk(stmt);
+ assertEquals(expectedHints, HintsToStrings(ctasStmt.getInsertStmt().getPlanHints()));
+ }
+
+ static private String[] HintsToStrings(List<PlanHint> hints) {
+ if (hints.isEmpty()) return new String[] { null };
+ String[] hintAsStrings = new String[hints.size()];
+ for (int i = 0; i < hints.size(); ++i) hintAsStrings[i] = hints.get(i).toString();
+ return hintAsStrings;
+ }
+
@Test
public void TestPlanHints() {
- // All plan-hint styles embed a comma-separated list of hints.
- String[][] hintStyles = new String[][] {
- new String[] { "/* +", "*/" }, // traditional commented hint
- new String[] { "-- +", "\n" }, // eol commented hint
- new String[] { "\n-- +", "\n" }, // eol commented hint
- new String[] { "[", "]" } // legacy style
- };
String[][] commentStyles = new String[][] {
new String[] { "/*", "*/" }, // traditional comment
new String[] { "--", "\n" } // eol comment
};
- for (String[] hintStyle: hintStyles) {
+ for (String[] hintStyle: hintStyles_) {
String prefix = hintStyle[0];
String suffix = hintStyle[1];
// Test join hints.
@@ -389,17 +405,14 @@ public class ParserTest extends FrontendTestBase {
"join %sshuffle%s functional.alltypes e using(string_col)",
suffix, suffix, suffix, suffix, prefix, "", "", ""));
- // Test insert hints.
- TestInsertStmtHints("insert %s into t %s select * from t",
- String.format("%snoshuffle%s", prefix, suffix), "noshuffle");
- TestInsertStmtHints("insert %s overwrite t %s select * from t",
- String.format("%snoshuffle%s", prefix, suffix), "noshuffle");
- TestInsertStmtHints("insert %s into t partition(x, y) %s select * from t",
- String.format("%snoshuffle%s", prefix, suffix), "noshuffle");
- TestInsertStmtHints("insert %s into t(a, b) partition(x, y) %s select * from t",
- String.format("%sshuffle%s", prefix, suffix), "shuffle");
- TestInsertStmtHints("insert %s overwrite t(a, b) partition(x, y) %s select * from t",
- String.format("%sfoo,bar,baz%s", prefix, suffix), "foo", "bar", "baz");
+ // Test insert/CTAS hints.
+ TestInsertAndCtasHints("t", "t", hintStyle, "noshuffle", "noshuffle");
+ TestInsertAndCtasHints("t partition(x, y)", "t partitioned by(x, y)",
+ hintStyle, "noshuffle", "noshuffle");
+ TestInsertAndCtasHints("t(a, b) partition(x, y)", "t partitioned by(x, y)",
+ hintStyle, "shuffle", "shuffle");
+ TestInsertAndCtasHints("t(a, b) partition(x, y)", "t partitioned by(x, y)",
+ hintStyle, "foo,bar,baz", "foo", "bar", "baz");
// Test upsert hints.
TestInsertStmtHints("upsert %s into t %s select * from t",
@@ -488,14 +501,29 @@ public class ParserTest extends FrontendTestBase {
ParserErrorOnInsertStmtHints("insert %s into t %s select * from t",
String.format("%shint_with_args( a , , ,,, b )%s", prefix, suffix));
- // Negative tests for hints cannot be specified at the both avilable locations.
+ TestInsertAndCtasHints("t", "t",
+ hintStyle, "hint_with_args(a)", "hint_with_args(a)");
+ TestInsertAndCtasHints("t", "t",
+ hintStyle, "clustered,shuffle,hint_with_args(a)",
+ "clustered", "shuffle", "hint_with_args(a)");
+ TestInsertAndCtasHints("t", "t",
+ hintStyle, "hint_with_args(a,b)", "hint_with_args(a,b)");
+ TestInsertAndCtasHints("t", "t",
+ hintStyle, "hint_with_args(a , b)", "hint_with_args(a,b)");
+
+ ParserErrorOnInsertStmtHints("insert %s into t %s select * from t",
+ String.format("%shint_with_args( a , , ,,, b )%s", prefix, suffix));
+ ParserError(String.format(
+ "create table t %shint_with_args( a , , ,,, b )%s as select * from t",
+ prefix, suffix));
+
+ // Negative tests for hints cannot be specified at the both available locations.
ParserError(String.format("insert %s into t %s select * from t",
String.format("%sshuffle%s", prefix, suffix),
String.format("%sclustered%s", prefix, suffix)));
ParserError(String.format("upsert %s into t %s select * from t",
String.format("%sshuffle%s", prefix, suffix),
String.format("%sclustered%s", prefix, suffix)));
-
}
// No "+" at the beginning so the comment is not recognized as a hint.
TestJoinHints("select * from functional.alltypes a join /* comment */" +
@@ -504,6 +532,8 @@ public class ParserTest extends FrontendTestBase {
(String) null);
TestInsertStmtHints("insert %s into t(a, b) partition(x, y) %s select 1",
"/* comment */", (String) null);
+ TestCtasHints("create /* comment */ table t partitioned by (x, y) as select 1",
+ (String) null);
TestSelectListHints("select /* -- +straight_join */ * from functional.alltypes",
(String) null);
TestSelectListHints("select /* abcdef +straight_join */ * from functional.alltypes",
http://git-wip-us.apache.org/repos/asf/impala/blob/07d9f71a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
index a813cce..760ea59 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
@@ -546,12 +546,7 @@ public class ToSqlTest extends FrontendTestBase {
*/
@Test
public void planHintsTest() {
- String[][] hintStyles = new String[][] {
- new String[] { "/* +", "*/" }, // traditional commented hint
- new String[] { "\n-- +", "\n" }, // eol commented hint
- new String[] { "[", "]" } // legacy style
- };
- for (String[] hintStyle: hintStyles) {
+ for (String[] hintStyle: hintStyles_) {
String prefix = hintStyle[0];
String suffix = hintStyle[1];
http://git-wip-us.apache.org/repos/asf/impala/blob/07d9f71a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
----------------------------------------------------------------------
diff --git a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
index 9be3343..28dc710 100644
--- a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
+++ b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
@@ -84,6 +84,11 @@ public class FrontendTestBase {
// Test-local list of test databases and tables. These are cleaned up in @After.
protected final List<Db> testDbs_ = Lists.newArrayList();
protected final List<Table> testTables_ = Lists.newArrayList();
+ protected final String[][] hintStyles_ = new String[][] {
+ new String[] { "/* +", "*/" }, // traditional commented hint
+ new String[] { "\n-- +", "\n" }, // eol commented hint
+ new String[] { "[", "]" } // legacy style
+ };
@BeforeClass
public static void setUp() throws Exception {
http://git-wip-us.apache.org/repos/asf/impala/blob/07d9f71a/testdata/workloads/functional-planner/queries/PlannerTest/ddl.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/ddl.test b/testdata/workloads/functional-planner/queries/PlannerTest/ddl.test
index 0129ad1..74c7e5f 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/ddl.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/ddl.test
@@ -204,6 +204,37 @@ WRITE TO HDFS [default.t, OVERWRITE=false, PARTITION-KEYS=(year,month)]
00:SCAN HDFS [functional.alltypes]
partitions=24/24 files=24 size=478.45KB
====
+# IMPALA-4167: if no (no)shuffle hint is given for CTAS into partitioned HDFS table, then
+# Impala is free to decide whether to add an exchange node or not. In this example, the
+# planner decides to shuffle. The aim of this test is to have a reference query where
+# 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
+select * from functional.alltypes
+---- DISTRIBUTEDPLAN
+WRITE TO HDFS [default.t, OVERWRITE=false, PARTITION-KEYS=(functional.alltypes.year,functional.alltypes.month)]
+| partitions=24
+|
+01:EXCHANGE [HASH(functional.alltypes.year,functional.alltypes.month)]
+|
+00:SCAN HDFS [functional.alltypes]
+ partitions=24/24 files=24 size=478.45KB
+====
+# IMPALA-4167: non-shuffled CTAS into partitioned table has no exchange node before write.
+# Note that plan hint tests for CTAS are minimal by design, as this logic is covered well
+# in insert.test.
+# 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 /*+ noshuffle, noclustered */ table t partitioned by (year, month) as
+select * from functional.alltypes
+---- DISTRIBUTEDPLAN
+WRITE TO HDFS [default.t, OVERWRITE=false, PARTITION-KEYS=(functional.alltypes.year,functional.alltypes.month)]
+| partitions=24
+|
+00:SCAN HDFS [functional.alltypes]
+ partitions=24/24 files=24 size=478.45KB
+====
# CTAS with more complex select query
create table t partitioned by (c_nationkey) sort by (c_custkey) as
select c_custkey, max(o_totalprice) as maxprice, c_nationkey
http://git-wip-us.apache.org/repos/asf/impala/blob/07d9f71a/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test b/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
index 835c41b..d506ae7 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
@@ -426,6 +426,27 @@ INSERT INTO KUDU [functional_kudu.alltypes]
00:SCAN HDFS [functional.alltypes]
partitions=24/24 files=24 size=478.45KB
====
+# Hint - noclustered should remove the sort node from CTAS.
+create /* +noclustered */ table t
+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
+select * from functional.alltypes;
+---- DISTRIBUTEDPLAN
+INSERT INTO KUDU [default.t]
+|
+00:SCAN HDFS [functional.alltypes]
+ partitions=24/24 files=24 size=478.45KB
+====
# IMPALA-5602: If a query contains predicates that are all pushed to kudu and there is a
# limit, then the query should not incorrectly run with 'small query' optimization.
select * from functional_kudu.alltypesagg where tinyint_col = 9 limit 10;