impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tarmstr...@apache.org
Subject incubator-impala git commit: IMPALA-3454: Kudu deletes may fail if subqueries are used
Date Wed, 25 May 2016 13:41:35 GMT
Repository: incubator-impala
Updated Branches:
  refs/heads/master 3ee075f96 -> f09c6311c


IMPALA-3454: Kudu deletes may fail if subqueries are used

During analysis the subquery is rewritten and during that process some
previous analysis state about the Kudu columns was lost and never
repopulated. If the Kudu table has keys of different data types that
could lead to an error about incorrect data types. It may also be
possible that if the data types did match, the wrong values would be
deleted.

Change-Id: I55b6fecfd35458fbb5bc20b4be1375484d7bc3c6
Reviewed-on: http://gerrit.cloudera.org:8080/2901
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal 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/f09c6311
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/f09c6311
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/f09c6311

Branch: refs/heads/master
Commit: f09c6311c9a05e590145c80c1f66e561c8a3ba61
Parents: 3ee075f
Author: casey <casey@cloudera.com>
Authored: Thu Apr 28 22:32:50 2016 -0700
Committer: Tim Armstrong <tarmstrong@cloudera.com>
Committed: Wed May 25 06:41:29 2016 -0700

----------------------------------------------------------------------
 .../cloudera/impala/analysis/DeleteStmt.java    |  6 ++--
 .../cloudera/impala/analysis/ModifyStmt.java    |  7 ++--
 .../cloudera/impala/analysis/UpdateStmt.java    |  6 ++--
 .../queries/QueryTest/kudu_crud.test            | 34 +++++++++++++++++++-
 4 files changed, 44 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f09c6311/fe/src/main/java/com/cloudera/impala/analysis/DeleteStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/com/cloudera/impala/analysis/DeleteStmt.java b/fe/src/main/java/com/cloudera/impala/analysis/DeleteStmt.java
index 0b589b2..d3967f4 100644
--- a/fe/src/main/java/com/cloudera/impala/analysis/DeleteStmt.java
+++ b/fe/src/main/java/com/cloudera/impala/analysis/DeleteStmt.java
@@ -53,8 +53,10 @@ public class DeleteStmt extends ModifyStmt {
   public DataSink createDataSink() {
     // analyze() must have been called before.
     Preconditions.checkState(table_ != null);
-    return TableSink.create(table_, TableSink.Op.DELETE, ImmutableList.<Expr>of(),
-        referencedColumns_, false, ignoreNotFound_);
+    TableSink tableSink = TableSink.create(table_, TableSink.Op.DELETE,
+        ImmutableList.<Expr>of(), referencedColumns_, false, ignoreNotFound_);
+    Preconditions.checkState(!referencedColumns_.isEmpty());
+    return tableSink;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f09c6311/fe/src/main/java/com/cloudera/impala/analysis/ModifyStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/com/cloudera/impala/analysis/ModifyStmt.java b/fe/src/main/java/com/cloudera/impala/analysis/ModifyStmt.java
index e269b62..d8dce60 100644
--- a/fe/src/main/java/com/cloudera/impala/analysis/ModifyStmt.java
+++ b/fe/src/main/java/com/cloudera/impala/analysis/ModifyStmt.java
@@ -79,14 +79,14 @@ public abstract class ModifyStmt extends StatementBase {
   // concrete table class. Result of analysis.
   protected KuduTable table_;
 
+  // END: Members that need to be reset()
+  /////////////////////////////////////////
+
   // Position mapping of output expressions of the sourceStmt_ to column indices in the
   // target table. The i'th position in this list maps to the referencedColumns_[i]'th
   // position in the target table. Set in createSourceStmt() during analysis.
   protected ArrayList<Integer> referencedColumns_;
 
-  // END: Members that need to be reset()
-  /////////////////////////////////////////
-
   // On tables with a primary key, ignore key not found errors.
   protected final boolean ignoreNotFound_;
 
@@ -163,7 +163,6 @@ public abstract class ModifyStmt extends StatementBase {
     fromClause_.reset();
     if (sourceStmt_ != null) sourceStmt_.reset();
     table_ = null;
-    referencedColumns_.clear();
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f09c6311/fe/src/main/java/com/cloudera/impala/analysis/UpdateStmt.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/com/cloudera/impala/analysis/UpdateStmt.java b/fe/src/main/java/com/cloudera/impala/analysis/UpdateStmt.java
index 3254827..cdb6b42 100644
--- a/fe/src/main/java/com/cloudera/impala/analysis/UpdateStmt.java
+++ b/fe/src/main/java/com/cloudera/impala/analysis/UpdateStmt.java
@@ -62,8 +62,10 @@ public class UpdateStmt extends ModifyStmt {
   public DataSink createDataSink() {
     // analyze() must have been called before.
     Preconditions.checkState(table_ != null);
-    return TableSink.create(table_, TableSink.Op.UPDATE, ImmutableList.<Expr>of(),
-        referencedColumns_, false, ignoreNotFound_);
+    DataSink dataSink = TableSink.create(table_, TableSink.Op.UPDATE,
+        ImmutableList.<Expr>of(), referencedColumns_, false, ignoreNotFound_);
+    Preconditions.checkState(!referencedColumns_.isEmpty());
+    return dataSink;
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f09c6311/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test b/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
index a451d79..d58c55e 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
@@ -255,4 +255,36 @@ insert into tdata values
 ---- QUERY
 delete ignore a from tdata a, tdata b where a.id = 666
 ---- RESULTS
-====
\ No newline at end of file
+====
+---- QUERY
+# IMPALA-3454: A delete that requires a rewrite may not get the Kudu column order correct
+# if the Kudu columns are of different types.
+create table impala_3454
+(key_1 tinyint, key_2 bigint)
+TBLPROPERTIES(
+'storage_handler' = 'com.cloudera.kudu.hive.KuduStorageHandler',
+'kudu.table_name' = 'impala_3454',
+'kudu.master_addresses' = '0.0.0.0:7051',
+'kudu.key_columns' = 'key_1,key_2'
+ )
+---- RESULTS
+====
+---- QUERY
+insert into impala_3454 values
+(1, 1),
+(2, 2),
+(3, 3)
+---- RESULTS
+: 3
+====
+---- QUERY
+delete from impala_3454 where key_1 < (select max(key_2) from impala_3454)
+---- RESULTS
+====
+---- QUERY
+select * from impala_3454
+---- RESULTS
+3,3
+---- TYPES
+TINYINT,BIGINT
+====


Mime
View raw message