IMPALA-5796: CTAS for Kudu fails with expr rewrite When an expr rewrite occurs, we reanalyze the statement. Some state that is set in TableDef::analyze() wasn't being reset() first, causing a failure during reanalysis.
This patch adds TableDef::reset(), which clears the TableDef state that is set during analyze(). Testing: - Added a regression test in AnalyzeDDLTest Change-Id: Ia67bb33736b5a843663b226cdd0fa5bd839cbea1 Reviewed-on: http://gerrit.cloudera.org:8080/7666 Reviewed-by: Thomas Tauber-Marshall <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/5fcc9cb4 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/5fcc9cb4 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/5fcc9cb4 Branch: refs/heads/master Commit: 5fcc9cb4cc2a9848c7d8ba677494588d8014fa92 Parents: 86e88ca Author: Thomas Tauber-Marshall <[email protected]> Authored: Mon Aug 14 11:39:03 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Thu Aug 17 20:01:27 2017 +0000 ---------------------------------------------------------------------- .../impala/analysis/CreateTableAsSelectStmt.java | 4 ++-- .../apache/impala/analysis/CreateTableStmt.java | 12 ++++++++++++ .../java/org/apache/impala/analysis/TableDef.java | 18 +++++++++++++++--- .../apache/impala/analysis/AnalyzeDDLTest.java | 3 +++ 4 files changed, 32 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5fcc9cb4/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 eb492c6..516a06e 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java @@ -49,8 +49,6 @@ import com.google.common.collect.Lists; * select statement. */ public class CreateTableAsSelectStmt extends StatementBase { - private final CreateTableStmt createStmt_; - // List of partition columns from the PARTITIONED BY (...) clause. Set to null if no // partition was given. private final List<String> partitionKeys_; @@ -58,6 +56,7 @@ public class CreateTableAsSelectStmt extends StatementBase { ///////////////////////////////////////// // BEGIN: Members that need to be reset() + private final CreateTableStmt createStmt_; private final InsertStmt insertStmt_; // END: Members that need to be reset() @@ -234,6 +233,7 @@ public class CreateTableAsSelectStmt extends StatementBase { @Override public void reset() { super.reset(); + createStmt_.reset(); insertStmt_.reset(); } } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5fcc9cb4/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java index 6169997..5810a40 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java @@ -51,9 +51,15 @@ public class CreateTableStmt extends StatementBase { final static String KUDU_STORAGE_HANDLER_ERROR_MESSAGE = "Kudu tables must be" + " specified using 'STORED AS KUDU'."; + ///////////////////////////////////////// + // BEGIN: Members that need to be reset() + // Table parameters specified in a CREATE TABLE statement private final TableDef tableDef_; + // END: Members that need to be reset() + ///////////////////////////////////////// + // Table owner. Set during analysis private String owner_; @@ -71,6 +77,12 @@ public class CreateTableStmt extends StatementBase { } @Override + public void reset() { + super.reset(); + tableDef_.reset(); + } + + @Override public CreateTableStmt clone() { return new CreateTableStmt(this); } public String getTbl() { return getTblName().getTbl(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5fcc9cb4/fe/src/main/java/org/apache/impala/analysis/TableDef.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/analysis/TableDef.java b/fe/src/main/java/org/apache/impala/analysis/TableDef.java index bd07af8..f4901f9 100644 --- a/fe/src/main/java/org/apache/impala/analysis/TableDef.java +++ b/fe/src/main/java/org/apache/impala/analysis/TableDef.java @@ -71,9 +71,6 @@ class TableDef { // mean no primary keys were specified as the columnDefs_ could contain primary keys. private final List<String> primaryKeyColNames_ = Lists.newArrayList(); - // Authoritative list of primary key column definitions populated during analysis. - private final List<ColumnDef> primaryKeyColDefs_ = Lists.newArrayList(); - // If true, the table's data will be preserved if dropped. private final boolean isExternal_; @@ -83,9 +80,18 @@ class TableDef { // Partitioning parameters. private final TableDataLayout dataLayout_; + ///////////////////////////////////////// + // BEGIN: Members that need to be reset() + + // Authoritative list of primary key column definitions populated during analysis. + private final List<ColumnDef> primaryKeyColDefs_ = Lists.newArrayList(); + // True if analyze() has been called. private boolean isAnalyzed_ = false; + // END: Members that need to be reset() + ///////////////////////////////////////// + /** * Set of table options. These options are grouped together for convenience while * parsing CREATE TABLE statements. They are typically found at the end of CREATE @@ -150,6 +156,11 @@ class TableDef { dataLayout_ = TableDataLayout.createEmptyLayout(); } + public void reset() { + primaryKeyColDefs_.clear(); + isAnalyzed_ = false; + } + public TableName getTblName() { return fqTableName_ != null ? fqTableName_ : tableName_; } @@ -191,6 +202,7 @@ class TableDef { * Analyzes the parameters of a CREATE TABLE statement. */ void analyze(Analyzer analyzer) throws AnalysisException { + if (isAnalyzed_) return; Preconditions.checkState(tableName_ != null && !tableName_.isEmpty()); fqTableName_ = analyzer.getFqTableName(getTblName()); fqTableName_.analyze(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5fcc9cb4/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 9e2ffd7..4d5ac0a 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java @@ -1591,6 +1591,9 @@ public class AnalyzeDDLTest extends FrontendTestBase { AnalyzesOk("create table t primary key(id) stored as kudu as select id, bool_col " + "from functional.alltypestiny", "Unpartitioned Kudu tables are inefficient for large data sizes."); + // IMPALA-5796: CTAS into a Kudu table with expr rewriting. + AnalyzesOk("create table t primary key(id) stored as kudu as select id, bool_col " + + "from functional.alltypestiny where id between 0 and 10"); // CTAS in an external Kudu table AnalysisError("create external table t stored as kudu " + "tblproperties('kudu.table_name'='t') as select id, int_col from " +
