From ba84ad03cb83d7f7aed8524fcfbb0e2cdc9fdd53 Mon Sep 17 00:00:00 2001 From: Thomas Tauber-Marshall Date: Mon, 30 Apr 2018 15:32:30 -0700 Subject: [PATCH] IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite This patch fixes two problems: - Previously a CTAS into a Kudu table where an expr rewrite occurred would create an unpartitioned table, due to the partition info being reset in TableDataLayout and then never reconstructed. Since the Kudu partition info is set by the parser and never changes, the solution is to not reset it. - Previously a CTAS into a Kudu table with a range partition where an expr rewrite occurred would fail with an analysis exception due to a Precondition check in RangePartition.analyze that checked that the RangePartition wasn't already analyzed, as the analysis can't be done twice. Since the state in RangePartition never changes, it doesn't need to be reanalyzed and we can just return instead of failing on the check. Testing: - Added an e2e test that creates a partitioned Kudu table with a CTAS with a rewrite, and checks that the expected partitions are created. Change-Id: I731743bd84cc695119e99342e1b155096147f0ed Reviewed-on: http://gerrit.cloudera.org:8080/10251 Reviewed-by: Alex Behm Tested-by: Impala Public Jenkins --- .../impala/analysis/CreateTableAsSelectStmt.java | 3 +++ .../org/apache/impala/analysis/RangePartition.java | 5 +++-- .../org/apache/impala/analysis/TableDataLayout.java | 5 ----- .../main/java/org/apache/impala/analysis/TableDef.java | 1 - .../queries/QueryTest/kudu_create.test | 10 ++++++++++ 5 files changed, 16 insertions(+), 8 deletions(-) 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 aac687369..5c8d9396f 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java @@ -253,6 +253,9 @@ public class CreateTableAsSelectStmt extends StatementBase { public void reset() { super.reset(); createStmt_.reset(); + // This is populated for CTAS in analyze(), so it needs to be cleared here. For other + // types of CreateTableStmts it is set by the parser and should not be reset. + createStmt_.getPartitionColumnDefs().clear(); insertStmt_.reset(); } } diff --git a/fe/src/main/java/org/apache/impala/analysis/RangePartition.java b/fe/src/main/java/org/apache/impala/analysis/RangePartition.java index caac462c7..9ed5da89e 100644 --- a/fe/src/main/java/org/apache/impala/analysis/RangePartition.java +++ b/fe/src/main/java/org/apache/impala/analysis/RangePartition.java @@ -121,8 +121,9 @@ public class RangePartition implements ParseNode { public void analyze(Analyzer analyzer, List partColDefs) throws AnalysisException { // Reanalyzing not supported because TIMESTAMPs are converted to BIGINT (unixtime - // micros) in place. - Preconditions.checkArgument(!isAnalyzed_); + // micros) in place. We can just return because none of the state will have changed + // since the first time we did the analysis. + if (isAnalyzed_) return; analyzeBoundaryValues(lowerBound_, partColDefs, analyzer); if (!isSingletonRange_) { analyzeBoundaryValues(upperBound_, partColDefs, analyzer); diff --git a/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java b/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java index fea809d4a..aef57320f 100644 --- a/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java +++ b/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java @@ -52,9 +52,4 @@ class TableDataLayout { List getPartitionColumnDefs() { return partitionColDefs_; } List getKuduPartitionParams() { return kuduPartitionParams_; } - - public void reset() { - partitionColDefs_.clear(); - kuduPartitionParams_.clear(); - } } 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 948515b85..529594bf6 100644 --- a/fe/src/main/java/org/apache/impala/analysis/TableDef.java +++ b/fe/src/main/java/org/apache/impala/analysis/TableDef.java @@ -161,7 +161,6 @@ class TableDef { public void reset() { primaryKeyColDefs_.clear(); - dataLayout_.reset(); columnDefs_.clear(); isAnalyzed_ = false; generatedKuduProperties_.clear(); diff --git a/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test b/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test index 394157c94..9d170556b 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test +++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test @@ -336,3 +336,13 @@ select * from ctas_decimal; ---- TYPES DECIMAL,DECIMAL,DECIMAL,DECIMAL,DECIMAL,DECIMAL ==== +---- QUERY +# IMPALA-6954: CTAS with an expr rewrite. +create table ctas_rewrite primary key(id) +partition by range(id) (partition 0 <= values < 100) stored as kudu +as select id, tinyint_col from functional.alltypes +where id between 0 and 1; +show range partitions ctas_rewrite; +---- RESULTS +'0 <= VALUES < 100' +==== \ No newline at end of file