Skip to content

Commit

Permalink
GH-44714: [C++] Keep field metadata for keys and values when importin…
Browse files Browse the repository at this point in the history
…g a map type via the C data interface (#44715)

### Rationale for this change

Import of a map type from the C data interface drops field metadata (including extension type information) which does not happen when importing a map type from IPC or a list of structs. This affects the ability to roundtrip data through pyarrow/Arrow C++ if extension types are not registered.

### What changes are included in this PR?

The mechanism to import the map type was changed to align with the method used for IPC import.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

The current behaviour was surprising/inconsistent, so I think this PR brings it in more line with the current expectation/documentation.
* GitHub Issue: #44714

Authored-by: Dewey Dunnington <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
  • Loading branch information
paleolimbot authored Nov 18, 2024
1 parent 59decc3 commit 152e878
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
8 changes: 4 additions & 4 deletions cpp/src/arrow/c/bridge.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1310,13 +1310,13 @@ struct SchemaImporter {
}

bool keys_sorted = (c_struct_->flags & ARROW_FLAG_MAP_KEYS_SORTED);
bool values_nullable = value_type->field(1)->nullable();

// Some implementations of Arrow (such as Rust) use a non-standard field name
// for key ("keys") and value ("values") fields. For simplicity, we override
// them on import.
auto values_field =
::arrow::field("value", value_type->field(1)->type(), values_nullable);
type_ = map(value_type->field(0)->type(), values_field, keys_sorted);
type_ =
std::make_shared<MapType>(value_type->field(0)->WithName("key"),
value_type->field(1)->WithName("value"), keys_sorted);
return Status::OK();
}

Expand Down
4 changes: 4 additions & 0 deletions cpp/src/arrow/c/bridge_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3769,6 +3769,10 @@ TEST_F(TestSchemaRoundtrip, RegisteredExtension) {
TEST_F(TestSchemaRoundtrip, Map) {
TestWithTypeFactory([&]() { return map(utf8(), int32()); });
TestWithTypeFactory([&]() { return map(utf8(), field("value", int32(), false)); });
TestWithTypeFactory([&]() {
return map(utf8(), field("value", int32(), false,
KeyValueMetadata::Make({"meta key"}, {"meta value"})));
});
// Field names are brought in line with the spec on import.
TestWithTypeFactory(
[&]() {
Expand Down

0 comments on commit 152e878

Please sign in to comment.