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 e0f96b9c8 KUDU-2671 forward-looking provision for AddRangePartition
e0f96b9c8 is described below
commit e0f96b9c838e33b93690a76f771d1eeaf3a99222
Author: Alexey Serbin <[email protected]>
AuthorDate: Thu Jul 7 17:03:55 2022 -0700
KUDU-2671 forward-looking provision for AddRangePartition
The way how the information on range-specific hash schema is specified
in AlterTableRequestPB::AddRangePartition introduced by [1] assumes
there should not be an empty custom hash schema for a range when the
table-wide hash schema isn't empty. As of now, the assumption holds
true since there is an artificial restriction on the variability of the
number of dimensions in per-range hash schemas in a table introduced
by changelist [2]. However, once the restriction introduced in [2] is
removed, the current type of the AddRangePartition::custom_hash_schema
deprives the code to tell between the case of a range-specific hash
schema with zero hash dimensions (a.k.a. empty hash schema, i.e. no hash
bucketing at all) and the case of table-wide hash schema for a newly
added range partition.
This patch fixes the deficiency, so now it's possible to call
has_custom_hash_schema() and hasCustomHashSchema() on AddRangePartition
object in C++ and Java code correspondingly, not relying on the
emptiness of the repeated field representing the set of hash dimensions
for the range-specific hash schema.
This patch would break backwards compatibility if there were a version
of Kudu released with the change introduced in changlist [1], but that's
not the case. So, it was possible to simply change the type of the
AddRangePartition::custom_hash_schema field.
[1]
https://github.com/apache/kudu/commit/11db3f28b36d92ce1515bcaace51a3586838abcb
[2]
https://github.com/apache/kudu/commit/6998193e69eeda497f912d1d806470c95b591ad4
Change-Id: I30f654443c7f51a76dea9d980588b399b06c2dd1
Reviewed-on: http://gerrit.cloudera.org:8080/18713
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Mahesh Reddy <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
.../java/org/apache/kudu/client/AlterTableOptions.java | 8 ++------
src/kudu/client/table_alterer-internal.cc | 2 +-
src/kudu/master/catalog_manager.cc | 7 ++++---
src/kudu/master/master-test.cc | 8 +++++---
src/kudu/master/master.proto | 14 ++++++++++++--
5 files changed, 24 insertions(+), 15 deletions(-)
diff --git
a/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
b/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
index 1603d0801..e5e8f193f 100644
---
a/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
+++
b/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
@@ -385,12 +385,8 @@ public class AlterTableOptions {
new Operation.OperationsEncoder().encodeLowerAndUpperBounds(
range.getLowerBound(), range.getUpperBound(),
range.getLowerBoundType(), range.getUpperBoundType()));
- for (org.apache.kudu.Common.PartitionSchemaPB.HashBucketSchemaPB
hashSchema :
- range.toPB().getHashSchemaList()) {
- Common.PartitionSchemaPB.HashBucketSchemaPB.Builder hbs =
- rangeBuilder.addCustomHashSchemaBuilder();
- hbs.mergeFrom(hashSchema);
- }
+ rangeBuilder.getCustomHashSchemaBuilder().addAllHashSchema(
+ range.toPB().getHashSchemaList());
step.setAddRangePartition(rangeBuilder);
if (!pb.hasSchema()) {
pb.setSchema(ProtobufHelper.schemaToPb(range.getLowerBound().getSchema(),
diff --git a/src/kudu/client/table_alterer-internal.cc
b/src/kudu/client/table_alterer-internal.cc
index 8e43ebfe2..68a8b8def 100644
--- a/src/kudu/client/table_alterer-internal.cc
+++ b/src/kudu/client/table_alterer-internal.cc
@@ -183,7 +183,7 @@ Status
KuduTableAlterer::Data::ToRequest(AlterTableRequestPB* req) {
for (const auto& hash_dimension : partition_data->hash_schema_) {
auto* custom_hash_schema_pb =
pb_step->mutable_add_range_partition()->
- add_custom_hash_schema();
+ mutable_custom_hash_schema()->add_hash_schema();
for (const auto& column_name : hash_dimension.column_names) {
custom_hash_schema_pb->add_columns()->set_name(column_name);
}
diff --git a/src/kudu/master/catalog_manager.cc
b/src/kudu/master/catalog_manager.cc
index 17411b1c3..534405aa6 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -2678,12 +2678,13 @@ Status CatalogManager::ApplyAlterPartitioningSteps(
const pair<KuduPartialRow, KuduPartialRow> range_bound =
{ *ops[0].split_row, *ops[1].split_row };
if (step.type() == AlterTableRequestPB::ADD_RANGE_PARTITION) {
- const auto& custom_hash_schema_pb =
- step.add_range_partition().custom_hash_schema();
- if (!FLAGS_enable_per_range_hash_schemas ||
custom_hash_schema_pb.empty()) {
+ if (!FLAGS_enable_per_range_hash_schemas ||
+ !step.add_range_partition().has_custom_hash_schema()) {
RETURN_NOT_OK(partition_schema.CreatePartitions(
{}, { range_bound }, schema, &partitions));
} else {
+ const auto& custom_hash_schema_pb =
+ step.add_range_partition().custom_hash_schema().hash_schema();
const Schema schema = client_schema.CopyWithColumnIds();
PartitionSchema::HashSchema hash_schema;
RETURN_NOT_OK(PartitionSchema::ExtractHashSchemaFromPB(
diff --git a/src/kudu/master/master-test.cc b/src/kudu/master/master-test.cc
index f66ddfc9b..7105b401d 100644
--- a/src/kudu/master/master-test.cc
+++ b/src/kudu/master/master-test.cc
@@ -976,7 +976,9 @@ TEST_P(AlterTableWithRangeSpecificHashSchema,
TestAlterTableWithDifferentHashDim
splits_encoder.Add(RowOperationsPB::RANGE_LOWER_BOUND, lower);
splits_encoder.Add(RowOperationsPB::RANGE_UPPER_BOUND, upper);
for (const auto& hash_dimension: custom_range_hash_schema) {
- auto* hash_dimension_pb =
step->mutable_add_range_partition()->add_custom_hash_schema();
+ auto* hash_dimension_pb =
+ step->mutable_add_range_partition()->mutable_custom_hash_schema()->
+ add_hash_schema();
for (const string& col_name: hash_dimension.columns) {
hash_dimension_pb->add_columns()->set_name(col_name);
}
@@ -1078,8 +1080,8 @@ TEST_F(MasterTest,
AlterTableAddAndDropRangeWithSpecificHashSchema) {
enc.Add(RowOperationsPB::RANGE_LOWER_BOUND, lower);
enc.Add(RowOperationsPB::RANGE_UPPER_BOUND, upper);
for (const auto& hash_dimension: custom_hash_schema) {
- auto* hash_dimension_pb =
- step->mutable_add_range_partition()->add_custom_hash_schema();
+ auto* hash_dimension_pb = step->mutable_add_range_partition()->
+ mutable_custom_hash_schema()->add_hash_schema();
for (const auto& col_name: hash_dimension.columns) {
hash_dimension_pb->add_columns()->set_name(col_name);
}
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index f3f65657a..e660a74df 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -725,6 +725,15 @@ message AlterTableRequestPB {
optional ColumnSchemaDeltaPB delta = 1;
}
message AddRangePartition {
+ // A structure to define range-specific hash schema. This separate type
+ // exists to distinguish from an empty hash schema (i.e. no hash bucketing)
+ // and the absence of range-specific hash schema when a range partition
+ // uses the table-wide hash schema instead. Otherwise, using a field of
+ // repeated HashBucketSchemaPB wouldn't allow to tell between those cases.
+ message CustomHashSchema {
+ repeated PartitionSchemaPB.HashBucketSchemaPB hash_schema = 1;
+ }
+
// A set of row operations containing the lower and upper range bound for
// the range partition to add or drop.
optional RowOperationsPB range_bounds = 1;
@@ -733,8 +742,9 @@ message AlterTableRequestPB {
// of the tablet's replicas.
optional string dimension_label = 2;
- // The custom hash partition schema for the range if specified.
- repeated PartitionSchemaPB.HashBucketSchemaPB custom_hash_schema = 3;
+ // The custom hash partition schema for the range, if specified. If absent,
+ // the range uses table-wide hash schema.
+ optional CustomHashSchema custom_hash_schema = 3;
}
message DropRangePartition {
// A set of row operations containing the lower and upper range bound for
