Skip to content

Commit ea0e701

Browse files
authored
GH-46146: [C++] Merge metadata in SchemaBuidler::AddMetadata (#46654)
### Rationale for this change `arrow::SchemaBuidler::AddMetadata()` replaces metadata not adds metadata. ### What changes are included in this PR? Add metadata when calling `SchemaBuidler::AddMetadata()`. ### Are these changes tested? Manually build pass. ### Are there any user-facing changes? **This PR includes breaking changes to public APIs.** `arrow::SchemaBuidler::AddMetadata()` adds not replaces the given metadata. If an existing code calls `AddMetadata()` multiple times, the behavior will be changed. * GitHub Issue: #46146 Authored-by: Ziy1-Tan <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
1 parent db2d831 commit ea0e701

File tree

2 files changed

+13
-6
lines changed

2 files changed

+13
-6
lines changed

cpp/src/arrow/type.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2641,7 +2641,8 @@ Status SchemaBuilder::AddSchemas(const std::vector<std::shared_ptr<Schema>>& sch
26412641
}
26422642

26432643
Status SchemaBuilder::AddMetadata(const KeyValueMetadata& metadata) {
2644-
impl_->metadata_ = metadata.Copy();
2644+
impl_->metadata_ =
2645+
impl_->metadata_ ? impl_->metadata_->Merge(metadata) : metadata.Copy();
26452646
return Status::OK();
26462647
}
26472648

cpp/src/arrow/type_test.cc

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -746,25 +746,31 @@ TEST(TestSchemaBuilder, WithMetadata) {
746746
auto f0 = field("f0", int32());
747747
auto f1 = field("f1", uint8(), false);
748748
auto metadata = key_value_metadata({{"foo", "bar"}});
749+
auto metadata2 = key_value_metadata({{"foo2", "bar2"}});
750+
auto merged_metadata = metadata->Merge(*metadata2);
749751

750752
SchemaBuilder builder;
751753
ASSERT_OK(builder.AddMetadata(*metadata));
752754
ASSERT_OK_AND_ASSIGN(auto schema, builder.Finish());
753755
AssertSchemaEqual(schema, ::arrow::schema({})->WithMetadata(metadata));
754756

757+
ASSERT_OK(builder.AddMetadata(*metadata2));
758+
ASSERT_OK_AND_ASSIGN(schema, builder.Finish());
759+
AssertSchemaEqual(schema, ::arrow::schema({})->WithMetadata(merged_metadata));
760+
755761
ASSERT_OK(builder.AddField(f0));
756762
ASSERT_OK_AND_ASSIGN(schema, builder.Finish());
757-
AssertSchemaEqual(schema, ::arrow::schema({f0})->WithMetadata(metadata));
763+
AssertSchemaEqual(schema, ::arrow::schema({f0})->WithMetadata(merged_metadata));
758764

759-
SchemaBuilder other_builder{::arrow::schema({})->WithMetadata(metadata)};
765+
SchemaBuilder other_builder{::arrow::schema({})->WithMetadata(merged_metadata)};
760766
ASSERT_OK(other_builder.AddField(f1));
761767
ASSERT_OK_AND_ASSIGN(schema, other_builder.Finish());
762-
AssertSchemaEqual(schema, ::arrow::schema({f1})->WithMetadata(metadata));
768+
AssertSchemaEqual(schema, ::arrow::schema({f1})->WithMetadata(merged_metadata));
763769

764770
other_builder.Reset();
765-
ASSERT_OK(other_builder.AddField(f1->WithMetadata(metadata)));
771+
ASSERT_OK(other_builder.AddField(f1->WithMetadata(merged_metadata)));
766772
ASSERT_OK_AND_ASSIGN(schema, other_builder.Finish());
767-
AssertSchemaEqual(schema, ::arrow::schema({f1->WithMetadata(metadata)}));
773+
AssertSchemaEqual(schema, ::arrow::schema({f1->WithMetadata(merged_metadata)}));
768774
}
769775

770776
TEST(TestSchemaBuilder, IncrementalConstruction) {

0 commit comments

Comments
 (0)