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/05/04 15:17:02 UTC

[5/8] impala git commit: IMPALA-6954: Fix problems with CTAS into Kudu with an expr rewrite

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 <al...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/bb4a59a7
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/bb4a59a7
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/bb4a59a7

Branch: refs/heads/2.x
Commit: bb4a59a73ca2d1fc93d503b25e560178a09627d8
Parents: 5ba4643
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
Authored: Mon Apr 30 15:32:30 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Thu May 3 19:59:25 2018 +0000

----------------------------------------------------------------------
 .../apache/impala/analysis/CreateTableAsSelectStmt.java   |  3 +++
 .../java/org/apache/impala/analysis/RangePartition.java   |  5 +++--
 .../java/org/apache/impala/analysis/TableDataLayout.java  |  5 -----
 fe/src/main/java/org/apache/impala/analysis/TableDef.java |  1 -
 .../functional-query/queries/QueryTest/kudu_create.test   | 10 ++++++++++
 5 files changed, 16 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/bb4a59a7/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 aac6873..5c8d939 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();
   }
 }

http://git-wip-us.apache.org/repos/asf/impala/blob/bb4a59a7/fe/src/main/java/org/apache/impala/analysis/RangePartition.java
----------------------------------------------------------------------
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 caac462..9ed5da8 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<ColumnDef> 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);

http://git-wip-us.apache.org/repos/asf/impala/blob/bb4a59a7/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
----------------------------------------------------------------------
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 fea809d..aef5732 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<ColumnDef> getPartitionColumnDefs() { return partitionColDefs_; }
   List<KuduPartitionParam> getKuduPartitionParams() { return kuduPartitionParams_; }
-
-  public void reset() {
-    partitionColDefs_.clear();
-    kuduPartitionParams_.clear();
-  }
 }

http://git-wip-us.apache.org/repos/asf/impala/blob/bb4a59a7/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 948515b..529594b 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();

http://git-wip-us.apache.org/repos/asf/impala/blob/bb4a59a7/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test b/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
index 394157c..9d17055 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