KUDU-1750: Drop range partition is not working as expected Change-Id: I82c11fbc767f81ab3fe8f486d76a45e3ca3f5b9a Reviewed-on: http://gerrit.cloudera.org:8080/5133 Reviewed-by: Adar Dembo <[email protected]> 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/b9453a25 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/b9453a25 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/b9453a25 Branch: refs/heads/master Commit: b9453a258711ff81d71c7435a0e2e949dbb16aa7 Parents: 7a4fafe Author: Dan Burkert <[email protected]> Authored: Thu Nov 17 16:44:47 2016 -0800 Committer: Dan Burkert <[email protected]> Committed: Fri Nov 18 01:58:29 2016 +0000 ---------------------------------------------------------------------- src/kudu/integration-tests/alter_table-test.cc | 194 ++++++++++++++++++++ src/kudu/master/catalog_manager.cc | 4 +- 2 files changed, 196 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/b9453a25/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 f82279d..a387b47 100644 --- a/src/kudu/integration-tests/alter_table-test.cc +++ b/src/kudu/integration-tests/alter_table-test.cc @@ -1304,6 +1304,200 @@ TEST_F(AlterTableTest, TestAlterRangePartitioningInvalid) { ASSERT_FALSE(s.ok()); ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); ASSERT_EQ(100, CountTableRows(table.get())); + + // KUDU-1750 Regression cases: + + // DROP [0, 50) <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 0)); + ASSERT_OK(upper->SetInt32("c0", 50)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // DROP [50, 100) <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 50)); + ASSERT_OK(upper->SetInt32("c0", 100)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // ADD [100, 200) + // DROP [100, 150) <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 100)); + ASSERT_OK(upper->SetInt32("c0", 200)); + table_alterer->AddRangePartition(lower.release(), upper.release()); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 100)); + ASSERT_OK(upper->SetInt32("c0", 150)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // ADD [100, 200) + // DROP [150, 200) <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 100)); + ASSERT_OK(upper->SetInt32("c0", 200)); + table_alterer->AddRangePartition(lower.release(), upper.release()); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 150)); + ASSERT_OK(upper->SetInt32("c0", 200)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // ADD [<min>, 0) + // DROP [<min>, -50] <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(upper->SetInt32("c0", 0)); + table_alterer->AddRangePartition(lower.release(), upper.release()); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(upper->SetInt32("c0", -50)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // ADD [<min>, 0) + // DROP [<min>, 50] <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(upper->SetInt32("c0", 0)); + table_alterer->AddRangePartition(lower.release(), upper.release()); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(upper->SetInt32("c0", 50)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // ADD [100, <max>) + // DROP [150, <max>] <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 100)); + table_alterer->AddRangePartition(lower.release(), upper.release()); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 150)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // ADD [100, <max>) + // DROP [50, <max>] <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 100)); + table_alterer->AddRangePartition(lower.release(), upper.release()); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 50)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // Setup for the next few test cases: + // ADD [<min>, 0) + // ADD [100, <max>) + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(upper->SetInt32("c0", 0)); + table_alterer->AddRangePartition(lower.release(), upper.release()); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 100)); + table_alterer->AddRangePartition(lower.release(), upper.release()); + ASSERT_OK(table_alterer->Alter()); + + // DROP [<min>, -50] <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(upper->SetInt32("c0", -50)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // DROP [<min>, 50] <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(upper->SetInt32("c0", 50)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // DROP [150, <max>] <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 150)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // DROP [50, <max>] <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + ASSERT_OK(lower->SetInt32("c0", 50)); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); + + // DROP [<min>, <max>] <- illegal + table_alterer.reset(client_->NewTableAlterer(table_name)); + lower.reset(schema_.NewRow()); + upper.reset(schema_.NewRow()); + table_alterer->DropRangePartition(lower.release(), upper.release()); + s = table_alterer->Alter(); + ASSERT_FALSE(s.ok()); + ASSERT_STR_CONTAINS(s.ToString(), "No range partition found for drop range partition step"); + ASSERT_EQ(100, CountTableRows(table.get())); } TEST_F(ReplicatedAlterTableTest, TestReplicatedAlter) { http://git-wip-us.apache.org/repos/asf/kudu/blob/b9453a25/src/kudu/master/catalog_manager.cc ---------------------------------------------------------------------- diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc index e617136..472316c 100644 --- a/src/kudu/master/catalog_manager.cc +++ b/src/kudu/master/catalog_manager.cc @@ -1412,12 +1412,12 @@ Status CatalogManager::ApplyAlterPartitioningSteps( if (existing_iter != existing_tablets.end()) { TabletMetadataLock metadata(existing_iter->second, TabletMetadataLock::READ); const auto& partition = metadata.data().pb.partition(); - found_existing = partition.partition_key_start() == lower_bound || + found_existing = partition.partition_key_start() == lower_bound && partition.partition_key_end() == upper_bound; } if (new_iter != new_tablets.end()) { const auto& partition = new_iter->second->mutable_metadata()->dirty().pb.partition(); - found_new = partition.partition_key_start() == lower_bound || + found_new = partition.partition_key_start() == lower_bound && partition.partition_key_end() == upper_bound; }
