KUDU-2191: reject alter table rename to same table name This commit ensures 'ALTER TABLE t RENAME TO t;' operations will fail validation in the catalog manager when the HMS integration is enabled. The HMS allows these types of alterations, however Kudu (without HMS integration) does not, so this extra validation aligns the behavior between the two configurations. Postgres also does not allow this type of alter.
Change-Id: I37d895bcc98cbc45a27ff25e97a25436273d79b0 Reviewed-on: http://gerrit.cloudera.org:8080/10831 Reviewed-by: Hao Hao <hao....@cloudera.com> Tested-by: Kudu Jenkins Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/29c66db4 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/29c66db4 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/29c66db4 Branch: refs/heads/master Commit: 29c66db48b635b7a1d708c7ddb7e0fcd3c8c33fd Parents: 72d0981 Author: Dan Burkert <danburk...@apache.org> Authored: Tue Jun 26 14:45:01 2018 -0700 Committer: Dan Burkert <danburk...@apache.org> Committed: Wed Jun 27 20:50:36 2018 +0000 ---------------------------------------------------------------------- src/kudu/integration-tests/alter_table-test.cc | 6 ++++++ src/kudu/integration-tests/master_hms-itest.cc | 10 ++++++++-- src/kudu/master/catalog_manager.cc | 12 ++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/29c66db4/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 640f1cb..6bc7929 100644 --- a/src/kudu/integration-tests/alter_table-test.cc +++ b/src/kudu/integration-tests/alter_table-test.cc @@ -804,6 +804,12 @@ TEST_F(AlterTableTest, TestRenameTableAndAdd) { ASSERT_OK(AddNewI32Column(new_name, "new", 0xdeadbeef)); } +TEST_F(AlterTableTest, TestRenameToSameName) { + unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer(kTableName)); + Status s = table_alterer->RenameTo(kTableName)->Alter(); + ASSERT_TRUE(s.IsAlreadyPresent()) << s.ToString(); +} + // Test restarting a tablet server several times after various // schema changes. // This is a regression test for KUDU-462. http://git-wip-us.apache.org/repos/asf/kudu/blob/29c66db4/src/kudu/integration-tests/master_hms-itest.cc ---------------------------------------------------------------------- diff --git a/src/kudu/integration-tests/master_hms-itest.cc b/src/kudu/integration-tests/master_hms-itest.cc index 160121d..84ed562 100644 --- a/src/kudu/integration-tests/master_hms-itest.cc +++ b/src/kudu/integration-tests/master_hms-itest.cc @@ -268,9 +268,15 @@ TEST_F(MasterHmsTest, TestRenameTable) { external_table.tableName = "b"; ASSERT_OK(hms_client_->CreateTable(external_table)); - // Attempt to rename the Kudu table to the external table name. + // Attempt to rename the Kudu table to the same name. unique_ptr<KuduTableAlterer> table_alterer(client_->NewTableAlterer("db.a")); - Status s = table_alterer->RenameTo("db.b")->Alter(); + Status s = table_alterer->RenameTo("db.a")->Alter(); + ASSERT_TRUE(s.IsAlreadyPresent()) << s.ToString(); + ASSERT_STR_CONTAINS(s.ToString(), "a already exists"); + + // Attempt to rename the Kudu table to the external table name. + table_alterer.reset(client_->NewTableAlterer("db.a")); + s = table_alterer->RenameTo("db.b")->Alter(); ASSERT_TRUE(s.IsIllegalState()) << s.ToString(); ASSERT_STR_CONTAINS(s.ToString(), "b already exists"); http://git-wip-us.apache.org/repos/asf/kudu/blob/29c66db4/src/kudu/master/catalog_manager.cc ---------------------------------------------------------------------- diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc index 49e368b..7094a70 100644 --- a/src/kudu/master/catalog_manager.cc +++ b/src/kudu/master/catalog_manager.cc @@ -2149,6 +2149,18 @@ Status CatalogManager::AlterTableRpc(const AlterTableRequestPB& req, RETURN_NOT_OK(FindAndLockTable(req, resp, LockMode::READ, &table, &l)); RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(&l, resp)); + // The HMS allows renaming a table to the same name (ALTER TABLE t RENAME TO t;), + // however Kudu does not, so we must validate this case ourselves. This + // invariant is also checked in CatalogManager::AlterTable, but when the HMS + // integration is enabled that check does not bubble up to the client (it's + // called by the notification log listener). + if (l.data().name() == req.new_table_name()) { + return SetupError( + Status::AlreadyPresent(Substitute("table $0 already exists with id $1", + req.new_table_name(), table->id())), + resp, MasterErrorPB::TABLE_ALREADY_PRESENT); + } + Schema schema; RETURN_NOT_OK(SchemaFromPB(l.data().pb.schema(), &schema));
