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 2017/08/18 15:12:50 UTC

[4/9] incubator-impala git commit: IMPALA-5796: CTAS for Kudu fails with expr rewrite

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 <tm...@cloudera.com>
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 <tm...@cloudera.com>
Authored: Mon Aug 14 11:39:03 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
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 " +