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 f40a39c [common] 'client_schema' isn't needed in
PartitionSchema::FromPB()
f40a39c is described below
commit f40a39c75709b0f2ecc714e45cc832906554fb8c
Author: Alexey Serbin <[email protected]>
AuthorDate: Mon Jul 19 23:26:50 2021 -0700
[common] 'client_schema' isn't needed in PartitionSchema::FromPB()
This patch removes unnecessary 'client_schema' parameter introduced
for PartitionSchema::FromPB() recently, also removing the overloaded
variant of the PartitionSchema::FromPB() method as well.
As far as I can see, neither ApplyAlterPartitioningSteps() nor
CreateTable() methods of the CatalogManager class needs to pass client
schema to PartitionSchema::FromPB(), and the tablet schema is a must in
both places anyway because column identifiers are required there.
Moreover, passing the client schema would only make the decoding slower
because of the logic to loop over columns while building projection
mapping in RowOperationsPBDecoder::DecodeOperations().
This is a follow-up to 2dda869456659a36247eb89f5b9e5e3837e5f8a3.
Change-Id: Iff5697bc51046052da13c758d79d8c659bc90711
Reviewed-on: http://gerrit.cloudera.org:8080/17704
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <[email protected]>
---
src/kudu/common/partition.cc | 9 +--------
src/kudu/common/partition.h | 7 -------
src/kudu/common/schema.cc | 1 +
src/kudu/master/catalog_manager.cc | 10 ++++++----
4 files changed, 8 insertions(+), 19 deletions(-)
diff --git a/src/kudu/common/partition.cc b/src/kudu/common/partition.cc
index ed6f56f..aa9e1fa 100644
--- a/src/kudu/common/partition.cc
+++ b/src/kudu/common/partition.cc
@@ -189,13 +189,6 @@ Status PartitionSchema::ExtractHashBucketSchemasFromPB(
Status PartitionSchema::FromPB(const PartitionSchemaPB& pb,
const Schema& schema,
PartitionSchema* partition_schema) {
- return FromPB(pb, schema, schema, partition_schema);
-}
-
-Status PartitionSchema::FromPB(const PartitionSchemaPB& pb,
- const Schema& schema,
- const Schema& client_schema,
- PartitionSchema* partition_schema) {
partition_schema->Clear();
RETURN_NOT_OK(ExtractHashBucketSchemasFromPB(schema,
pb.hash_bucket_schemas(),
&partition_schema->hash_bucket_schemas_));
@@ -207,7 +200,7 @@ Status PartitionSchema::FromPB(const PartitionSchemaPB& pb,
}
vector<pair<KuduPartialRow, KuduPartialRow>> range_bounds;
for (int i = 0; i < pb.range_bounds_size(); i++) {
- RowOperationsPBDecoder decoder(&pb.range_bounds(i), &client_schema,
&schema, nullptr);
+ RowOperationsPBDecoder decoder(&pb.range_bounds(i), &schema, &schema,
nullptr);
vector<DecodedRowOperation> ops;
RETURN_NOT_OK(decoder.DecodeOperations<DecoderMode::SPLIT_ROWS>(&ops));
if (ops.size() != 2) {
diff --git a/src/kudu/common/partition.h b/src/kudu/common/partition.h
index ff58fb0..82a4a43 100644
--- a/src/kudu/common/partition.h
+++ b/src/kudu/common/partition.h
@@ -175,13 +175,6 @@ class PartitionSchema {
const Schema& schema,
PartitionSchema* partition_schema) WARN_UNUSED_RESULT;
- // Overloaded function similar to function above, used when an
- // explicit client schema is available to decode the range bounds.
- static Status FromPB(const PartitionSchemaPB& pb,
- const Schema& schema,
- const Schema& client_schema,
- PartitionSchema* partition_schema) WARN_UNUSED_RESULT;
-
// Serializes a partition schema into a protobuf message.
// Requires a schema to encode the range bounds.
Status ToPB(const Schema& schema, PartitionSchemaPB* pb) const;
diff --git a/src/kudu/common/schema.cc b/src/kudu/common/schema.cc
index e723bc4..38c0ce5 100644
--- a/src/kudu/common/schema.cc
+++ b/src/kudu/common/schema.cc
@@ -374,6 +374,7 @@ Status Schema::CreateProjectionByIdsIgnoreMissing(const
std::vector<ColumnId>& c
Schema Schema::CopyWithColumnIds() const {
CHECK(!has_column_ids());
vector<ColumnId> ids;
+ ids.reserve(num_columns());
for (int32_t i = 0; i < num_columns(); i++) {
ids.emplace_back(kFirstColumnId + i);
}
diff --git a/src/kudu/master/catalog_manager.cc
b/src/kudu/master/catalog_manager.cc
index 174a483..12df2a6 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -1806,14 +1806,14 @@ Status CatalogManager::CreateTable(const
CreateTableRequestPB* orig_req,
return SetupError(Status::InvalidArgument("user requests should not have
Column IDs"),
resp, MasterErrorPB::INVALID_SCHEMA);
}
- Schema schema = client_schema.CopyWithColumnIds();
+ const Schema schema = client_schema.CopyWithColumnIds();
// If the client did not set a partition schema in the create table request,
// the default partition schema (no hash bucket components and a range
// partitioned on the primary key columns) will be used.
PartitionSchema partition_schema;
RETURN_NOT_OK(SetupError(
- PartitionSchema::FromPB(req.partition_schema(), schema, client_schema,
&partition_schema),
+ PartitionSchema::FromPB(req.partition_schema(), schema,
&partition_schema),
resp, MasterErrorPB::INVALID_SCHEMA));
// Decode split rows.
@@ -2592,11 +2592,13 @@ Status CatalogManager::ApplyAlterPartitioningSteps(
vector<scoped_refptr<TabletInfo>>* tablets_to_add,
vector<scoped_refptr<TabletInfo>>* tablets_to_drop) {
+ // Get the table's schema as it's known to the catalog manager.
Schema schema;
RETURN_NOT_OK(SchemaFromPB(l.data().pb.schema(), &schema));
+ // Build current PartitionSchema for the table.
PartitionSchema partition_schema;
- RETURN_NOT_OK(PartitionSchema::FromPB(l.data().pb.partition_schema(), schema,
- client_schema, &partition_schema));
+ RETURN_NOT_OK(PartitionSchema::FromPB(
+ l.data().pb.partition_schema(), schema, &partition_schema));
TableInfo::TabletInfoMap existing_tablets = table->tablet_map();
TableInfo::TabletInfoMap new_tablets;
