You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2017/10/02 19:09:06 UTC
[2/2] kudu git commit: KUDU-2132 Error dropping columns renamed to
old key column names
KUDU-2132 Error dropping columns renamed to old key column names
Suppose a table has two columns 'a' and 'b', and 'a' is the primary
key. Consider the following sequence of alter table steps, done as
part of one alter table request:
1. Rename the key column 'a' to 'c'.
2. Rename 'b' to 'a', the previous name of the key column.
3. Drop 'a'.
This is a valid sequence, but previously we were rejecting it,
saying that a primary key column was being dropped, even though
that isn't the case.
The problem is that CatalogManager::ApplyAlterSchemaSteps
checked whether a column was a key column by checking by
name against the table's schema before any alter steps were
applied. The fix is to check against the schema with the
previous alter steps applied.
Change-Id: Ie27f7aa936e6dfcf6c8b0b7a701784b0683680f2
Reviewed-on: http://gerrit.cloudera.org:8080/8188
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Dan Burkert <da...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/5ca7deae
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/5ca7deae
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/5ca7deae
Branch: refs/heads/master
Commit: 5ca7deae7483d9e6427d795b806680ec3c08a73c
Parents: 3ec8834
Author: Will Berkeley <wd...@apache.org>
Authored: Fri Sep 29 15:17:17 2017 -0700
Committer: Will Berkeley <wd...@gmail.com>
Committed: Mon Oct 2 18:58:16 2017 +0000
----------------------------------------------------------------------
src/kudu/common/schema.h | 10 ++++++++++
src/kudu/integration-tests/alter_table-test.cc | 9 +++++++++
src/kudu/master/catalog_manager.cc | 2 +-
src/kudu/master/catalog_manager.h | 1 +
4 files changed, 21 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/5ca7deae/src/kudu/common/schema.h
----------------------------------------------------------------------
diff --git a/src/kudu/common/schema.h b/src/kudu/common/schema.h
index d9fe0ab..724b9c0 100644
--- a/src/kudu/common/schema.h
+++ b/src/kudu/common/schema.h
@@ -891,6 +891,16 @@ class SchemaBuilder {
return next_id_;
}
+ // Return true if the column named 'col_name' is a key column.
+ // Return false if the column is not a key column, or if
+ // the column is not in the builder.
+ bool is_key_column(const StringPiece col_name) const {
+ for (int i = 0; i < num_key_columns_; i++) {
+ if (cols_[i].name() == col_name) return true;
+ }
+ return false;
+ }
+
Schema Build() const { return Schema(cols_, col_ids_, num_key_columns_); }
Schema BuildWithoutIds() const { return Schema(cols_, num_key_columns_); }
http://git-wip-us.apache.org/repos/asf/kudu/blob/5ca7deae/src/kudu/integration-tests/alter_table-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/alter_table-test.cc b/src/kudu/integration-tests/alter_table-test.cc
index bc2a501..55caef0 100644
--- a/src/kudu/integration-tests/alter_table-test.cc
+++ b/src/kudu/integration-tests/alter_table-test.cc
@@ -2102,4 +2102,13 @@ TEST_F(AlterTableTest, TestRenameStillCreatingTable) {
}
}
+// Regression test for KUDU-2132.
+TEST_F(AlterTableTest, TestKUDU2132) {
+ unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer(kTableName));
+ table_alterer->AlterColumn("c0")->RenameTo("primaryKeyRenamed");
+ table_alterer->AlterColumn("c1")->RenameTo("c0");
+ table_alterer->DropColumn("c0");
+ ASSERT_OK(table_alterer->Alter());
+}
+
} // namespace kudu
http://git-wip-us.apache.org/repos/asf/kudu/blob/5ca7deae/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 7c95d43..e073a97 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -1656,7 +1656,7 @@ Status CatalogManager::ApplyAlterSchemaSteps(const SysTablesEntryPB& current_pb,
return Status::InvalidArgument("DROP_COLUMN missing column info");
}
- if (cur_schema.is_key_column(step.drop_column().name())) {
+ if (builder.is_key_column(step.drop_column().name())) {
return Status::InvalidArgument("cannot remove a key column",
step.drop_column().name());
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/5ca7deae/src/kudu/master/catalog_manager.h
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.h b/src/kudu/master/catalog_manager.h
index f2b1798..0a06912 100644
--- a/src/kudu/master/catalog_manager.h
+++ b/src/kudu/master/catalog_manager.h
@@ -22,6 +22,7 @@
#include <iosfwd>
#include <map>
#include <memory>
+#include <mutex>
#include <set>
#include <string>
#include <unordered_map>