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 <[email protected]> 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 <[email protected]> Authored: Thu Apr 28 22:32:50 2016 -0700 Committer: Tim Armstrong <[email protected]> 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 +====
