You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ph...@apache.org on 2018/02/02 18:51:28 UTC

[04/19] impala git commit: IMPALA-6454: CTAS into Kudu fails with mixed-case partition or primary key column names.

IMPALA-6454: CTAS into Kudu fails with mixed-case partition or primary key column names.

CTAS into Kudu fails if the primary key and/or the partition column names are not
specified in lower case.The problem is that we pass in the primary key column names
directly from the parser instead we should be passing the post-analysis ColumnDefs
as primary keys. So it is fixed by making changes to createCtasTarget()
in KuduTable class to take a list of ColumnDef class associated with the primary keys
from getPrimaryKeyColumnDefs() in CreateTableStmt class.

ColumnDef class has column names stored in lower case that are used by createCtasTarget()
to populate primaryKeyColumnNames_ in KuduTable class that resolves the issue.

Testing
-------
Verified against the newly added test case that reproduces the issue without the fix.

Change-Id: Ica1c8ec1544339e9e80733a7a0c78594e0a727d2
Reviewed-on: http://gerrit.cloudera.org:8080/9147
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/2.x
Commit: 530fa27c5cb9fc327c0b2c1518dc7edc1eb5c4fa
Parents: a326d51
Author: Pranay <ps...@cloudera.com>
Authored: Fri Jan 26 18:36:59 2018 -0800
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Fri Feb 2 01:10:15 2018 +0000

----------------------------------------------------------------------
 .../org/apache/impala/analysis/CreateTableAsSelectStmt.java  | 2 +-
 .../java/org/apache/impala/analysis/CreateTableStmt.java     | 2 +-
 fe/src/main/java/org/apache/impala/catalog/KuduTable.java    | 8 +++++---
 .../test/java/org/apache/impala/analysis/AnalyzeDDLTest.java | 7 +++++++
 4 files changed, 14 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/530fa27c/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 ba9a02a..e050d06 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
@@ -191,7 +191,7 @@ public class CreateTableAsSelectStmt extends StatementBase {
       Table tmpTable = null;
       if (KuduTable.isKuduTable(msTbl)) {
         tmpTable = KuduTable.createCtasTarget(db, msTbl, createStmt_.getColumnDefs(),
-            createStmt_.getTblPrimaryKeyColumnNames(),
+            createStmt_.getPrimaryKeyColumnDefs(),
             createStmt_.getKuduPartitionParams());
       } else {
         // TODO: Creating a tmp table using load() is confusing.

http://git-wip-us.apache.org/repos/asf/impala/blob/530fa27c/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 b4d59e8..2e5425a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
@@ -94,7 +94,7 @@ public class CreateTableStmt extends StatementBase {
     getColumnDefs().clear();
     getColumnDefs().addAll(colDefs);
   }
-  private List<ColumnDef> getPrimaryKeyColumnDefs() {
+  public List<ColumnDef> getPrimaryKeyColumnDefs() {
     return tableDef_.getPrimaryKeyColumnDefs();
   }
   public boolean isExternal() { return tableDef_.isExternal(); }

http://git-wip-us.apache.org/repos/asf/impala/blob/530fa27c/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/KuduTable.java b/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
index e9e1617..8296ed0 100644
--- a/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/KuduTable.java
@@ -107,7 +107,7 @@ public class KuduTable extends Table {
   // Comma separated list of Kudu master hosts with optional ports.
   private String kuduMasters_;
 
-  // Primary key column names.
+  // Primary key column names, the column names are all in lower case.
   private final List<String> primaryKeyColumnNames_ = Lists.newArrayList();
 
   // Partitioning schemes of this Kudu table. Both range and hash-based partitioning are
@@ -318,13 +318,15 @@ public class KuduTable extends Table {
    */
   public static KuduTable createCtasTarget(Db db,
       org.apache.hadoop.hive.metastore.api.Table msTbl, List<ColumnDef> columnDefs,
-      List<String> primaryKeyColumnNames, List<KuduPartitionParam> partitionParams) {
+      List<ColumnDef> primaryKeyColumnDefs, List<KuduPartitionParam> partitionParams) {
     KuduTable tmpTable = new KuduTable(msTbl, db, msTbl.getTableName(), msTbl.getOwner());
     int pos = 0;
     for (ColumnDef colDef: columnDefs) {
       tmpTable.addColumn(new Column(colDef.getColName(), colDef.getType(), pos++));
     }
-    tmpTable.primaryKeyColumnNames_.addAll(primaryKeyColumnNames);
+    for (ColumnDef pkColDef: primaryKeyColumnDefs) {
+      tmpTable.primaryKeyColumnNames_.add(pkColDef.getColName());
+    }
     tmpTable.partitionBy_.addAll(partitionParams);
     return tmpTable;
   }

http://git-wip-us.apache.org/repos/asf/impala/blob/530fa27c/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 3083f1f..122b49d 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
@@ -1733,6 +1733,13 @@ public class AnalyzeDDLTest extends FrontendTestBase {
         " stored as kudu as select id, a from functional.complextypes_fileformat",
         "Expr 'a' in select list returns a complex type 'ARRAY<INT>'.\n" +
         "Only scalar types are allowed in the select list.");
+
+    // IMPALA-6454: CTAS into Kudu tables with primary key specified in upper case.
+    AnalyzesOk("create table part_kudu_tbl primary key(INT_COL, SMALLINT_COL, ID)" +
+        " partition by hash(INT_COL, SMALLINT_COL, ID) PARTITIONS 2" +
+        " stored as kudu as SELECT INT_COL, SMALLINT_COL, ID, BIGINT_COL," +
+        " DATE_STRING_COL, STRING_COL, TIMESTAMP_COL, YEAR, MONTH FROM " +
+        " functional.alltypes");
   }
 
   @Test