This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push: new ce499cec3 KUDU-2671 fix the check for hash dimensions uniformity ce499cec3 is described below commit ce499cec329874db03317ad57a165959fb8893b4 Author: Alexey Serbin <ale...@apache.org> AuthorDate: Mon May 16 17:23:09 2022 -0700 KUDU-2671 fix the check for hash dimensions uniformity Before this patch, it was possible to create a table without ranges using the table-wide hash schema and ranges with custom hash schemas even if the table-wide hash schema had number of hash dimensions different from per-range custom hash schemas. This patch fixes the bug. I added a corresponding test scenario, updated existing ones, and removed a duplicate one. This is a follow-up to 6998193e69eeda497f912d1d806470c95b591ad4. Change-Id: I8c1282973deba57269f6e962be77e27baa39b187 Reviewed-on: http://gerrit.cloudera.org:8080/18532 Reviewed-by: Mahesh Reddy <mre...@cloudera.com> Tested-by: Alexey Serbin <ale...@apache.org> Reviewed-by: Attila Bukor <abu...@apache.org> --- src/kudu/client/flex_partitioning_client-test.cc | 89 ++++++++++++++++-------- src/kudu/master/catalog_manager.cc | 5 +- 2 files changed, 64 insertions(+), 30 deletions(-) diff --git a/src/kudu/client/flex_partitioning_client-test.cc b/src/kudu/client/flex_partitioning_client-test.cc index e8e97e4ba..0c72faa2e 100644 --- a/src/kudu/client/flex_partitioning_client-test.cc +++ b/src/kudu/client/flex_partitioning_client-test.cc @@ -270,29 +270,6 @@ class FlexPartitioningTest : public KuduTest { // specified when creating a table. class FlexPartitioningCreateTableTest : public FlexPartitioningTest {}; -// Create tables with range partitions using custom hash bucket schemas only. -// -// TODO(aserbin): turn the sub-scenarios with non-primary-key columns for -// custom hash buckets into negative ones after proper -// checks are added at the server side -// TODO(aserbin): add verification based on PartitionSchema provided by -// KuduTable::partition_schema() once PartitionPruner -// recognized custom hash bucket schema for ranges -TEST_F(FlexPartitioningCreateTableTest, CustomHashSchema) { - // One-level hash bucket structure: { 3, "key" }. - { - constexpr const char* const kTableName = "3@key"; - RangePartitions partitions; - partitions.emplace_back(CreateRangePartition(0, 100)); - auto& p = partitions.back(); - ASSERT_OK(p->add_hash_partitions({ kKeyColumn }, 3, 0)); - ASSERT_OK(CreateTable(kTableName, std::move(partitions))); - NO_FATALS(CheckTabletCount(kTableName, 3)); - ASSERT_OK(InsertTestRows(kTableName, 0, 100)); - NO_FATALS(CheckTableRowsNum(kTableName, 100)); - } -} - TEST_F(FlexPartitioningCreateTableTest, TableWideHashSchema) { // Create a table with the following partitions: // @@ -359,6 +336,23 @@ TEST_F(FlexPartitioningCreateTableTest, EmptyTableWideHashSchema) { //NO_FATALS(CheckTableRowsNum(kTableName, 333)); } +// Create tables with range partitions using custom hash bucket schemas only. +TEST_F(FlexPartitioningCreateTableTest, CustomHashSchemaDiffersFromTableWide) { + // Using one-level hash bucketing { 3, "key" } as the custom hash schema + // for the newly created range partition. Note that the table-wide hash schema + // is empty per FlexPartitioningTest::FlexPartitioningTest(), so the attempt + // to create such a table fails with the Status::NotSupported() status. + constexpr const char* const kTableName = "3@key"; + RangePartitions partitions; + partitions.emplace_back(CreateRangePartition(0, 100)); + auto& p = partitions.back(); + ASSERT_OK(p->add_hash_partitions({ kKeyColumn }, 3, 0)); + const auto s = CreateTable(kTableName, std::move(partitions)); + ASSERT_TRUE(s.IsNotSupported()) << s.ToString(); + ASSERT_STR_CONTAINS(s.ToString(), + "varying number of hash dimensions per range is not yet supported"); +} + // TODO(aserbin): re-enable this scenario once varying hash dimensions per range // are supported TEST_F(FlexPartitioningCreateTableTest, DISABLED_SingleCustomRangeEmptyHashSchema) { @@ -579,6 +573,7 @@ std::ostream& operator<<(std::ostream& os, struct PerRangeTestParameters { bool allowed; TablePartitionParameters partition_params; + vector<HashDimensionParameters> table_wide_hash_schema_params; }; std::ostream& operator<<(std::ostream& os, const PerRangeTestParameters& params) { @@ -619,6 +614,11 @@ class FlexPartitioningCreateTableParamTest : table_creator->add_custom_range_partition(p.release()); } + for (const auto& table_wide_params : params.table_wide_hash_schema_params) { + table_creator->add_hash_partitions(table_wide_params.columns, + table_wide_params.num_buckets); + } + if (params.allowed) { ASSERT_OK(table_creator->Create()); } else { @@ -636,13 +636,17 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { true, { { { INT32_MIN, 0 }, {} }, - } + }, + {} // empty table-wide hash schema }, // A single range with minimal hash schema. { true, { { { INT32_MIN, 0 }, { { { kKeyColumn }, 2, } } }, + }, + { + { { kKeyColumn }, 3 }, } }, // A single range with hashing on both columns of the primary key. @@ -650,6 +654,9 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { true, { { { INT32_MIN, 0 }, { { { kKeyColumn, kIntValColumn }, 2, } } }, + }, + { + { { kKeyColumn }, 5 }, } }, // Varying the number of hash buckets. @@ -660,6 +667,9 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { { { 100, 200 }, { { { kKeyColumn }, 3, } } }, { { 200, 300 }, { { { kKeyColumn }, 2, } } }, { { 300, 400 }, { { { kKeyColumn }, 10, } } }, + }, + { + { { kIntValColumn }, 5 }, } }, // Varying the set of columns for hash bucketing. @@ -670,6 +680,9 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { { { 200, 300 }, { { { kKeyColumn, kIntValColumn }, 2, } } }, { { 300, 400 }, { { { kIntValColumn }, 2, } } }, { { 400, 500 }, { { { kIntValColumn, kKeyColumn }, 2, } } }, + }, + { + { { kIntValColumn }, 3 }, } }, // Varying the set of columns for hash bucketing and number of buckets. @@ -683,6 +696,9 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { { { 600, 700 }, { { { kKeyColumn }, 3, } } }, { { 700, 800 }, { { { kIntValColumn, kKeyColumn }, 16, } } }, { { 800, 900 }, { { { kIntValColumn }, 32, } } }, + }, + { + { { kIntValColumn }, 7 }, } }, // Below are multiple scenarios with varying number of hash dimensions @@ -692,13 +708,17 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { { { { INT32_MIN, 0 }, {} }, { { 0, 100 }, { { { kKeyColumn }, 2 } } }, - } + }, + {} // an empty table-wide schema }, { false, { { { INT32_MIN, 0 }, { { { kKeyColumn }, 3 } } }, { { 0, 100 }, {} }, + }, + { + { { kKeyColumn }, 8 }, } }, { @@ -707,7 +727,8 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { { { INT32_MIN, 0 }, {} }, { { 0, 100 }, { { { kKeyColumn }, 2 } } }, { { 100, 200 }, {} }, - } + }, + {} // an empty table-wide schema }, { false, @@ -715,13 +736,18 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { { { INT32_MIN, 0 }, { { { kKeyColumn }, 3 } } }, { { 0, 100 }, {} }, { { 100, 200 }, { { { kIntValColumn }, 2 } } }, - } + }, + {} // an empty table-wide schema }, { false, { { { 0, 100 }, { { { kKeyColumn }, 2 }, { { kIntValColumn }, 2 } } }, { { 100, 200 }, { { { kKeyColumn, kIntValColumn }, 2 } } }, + }, + { + { { kKeyColumn }, 7 }, + { { kIntValColumn }, 3 }, } }, { @@ -729,6 +755,9 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { { { { 100, 200 }, { { { kKeyColumn }, 2 }, { { kIntValColumn }, 3 } } }, { { 200, 300 }, { { { kKeyColumn }, 3 } } }, + }, + { + { { kKeyColumn }, 7 }, } }, { @@ -736,6 +765,9 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { { { { 300, 400 }, { { { kKeyColumn }, 2 }, { { kIntValColumn }, 3 } } }, { { 400, 500 }, { { { kKeyColumn }, 3 } } }, + }, + { + { { kKeyColumn }, 7 }, } }, { @@ -744,7 +776,8 @@ const vector<PerRangeTestParameters> kPerRangeTestParameters = { { { 500, 600 }, { { { kKeyColumn }, 10 }, { { kIntValColumn }, 5 } } }, { { 600, 700 }, { { { kKeyColumn }, 2 } } }, { { 700, INT32_MAX }, {} }, - } + }, + {} // an empty table-wide schema }, }; INSTANTIATE_TEST_SUITE_P(Variations, FlexPartitioningCreateTableParamTest, diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc index a1cf87ac8..2928466f4 100644 --- a/src/kudu/master/catalog_manager.cc +++ b/src/kudu/master/catalog_manager.cc @@ -1943,12 +1943,13 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req, split_rows, range_bounds, range_hash_schemas, schema, &partitions)); // Check the restriction on the same number of hash dimensions across all the - // ranges. + // ranges. Also, check that the table-wide hash schema has the same number + // of hash dimensions as all the partitions with custom hash schemas. // // TODO(aserbin): remove the restriction once the rest of the code is ready // to handle range partitions with arbitrary hash schemas CHECK(!partitions.empty()); - const auto hash_dimensions_num = partitions.begin()->hash_buckets().size(); + const auto hash_dimensions_num = partition_schema.hash_schema().size(); for (const auto& p : partitions) { if (p.hash_buckets().size() != hash_dimensions_num) { return Status::NotSupported(