[arrow] branch main updated: GH-34983: [C++] Preserve map values nullability on C Data Interface import (#35013)

[wjones127]

This is an automated email from the ASF dual-hosted git repository.

wjones127 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new c7ba0cd2cb GH-34983: [C++] Preserve map values nullability on C Data 
Interface import (#35013)
c7ba0cd2cb is described below

commit c7ba0cd2cbdd89bce7d58edf573a9d3c16cf647d
Author: Will Jones <willjones...@gmail.com>
AuthorDate: Mon Apr 10 18:40:00 2023 -0700

    GH-34983: [C++] Preserve map values nullability on C Data Interface import 
(#35013)
    
    ### Rationale for this change
    
    Map types with non-nullable values don't roundtrip right now through C Data 
interface, when they arguably should.
    
    I also found that field names don't roundtrip, but I think that might be a 
feature, not a bug. Therefore I added unit tests enforcing it for now.
    
    ### What changes are included in this PR?
    
    ### Are these changes tested?
    
    Yes, added various unit tests for export and roundtrip.
    
    ### Are there any user-facing changes?
    
    * Closes: #34983
    
    Authored-by: Will Jones <willjones...@gmail.com>
    Signed-off-by: Will Jones <willjones...@gmail.com>
---
 cpp/src/arrow/c/bridge.cc      |  8 +++++++-
 cpp/src/arrow/c/bridge_test.cc | 15 +++++++++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/cpp/src/arrow/c/bridge.cc b/cpp/src/arrow/c/bridge.cc
index d6ea60f520..e1dddbaf6a 100644
--- a/cpp/src/arrow/c/bridge.cc
+++ b/cpp/src/arrow/c/bridge.cc
@@ -1102,7 +1102,13 @@ struct SchemaImporter {
     }
 
     bool keys_sorted = (c_struct_->flags & ARROW_FLAG_MAP_KEYS_SORTED);
-    type_ = map(value_type->field(0)->type(), value_type->field(1)->type(), 
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);
     return Status::OK();
   }
 
diff --git a/cpp/src/arrow/c/bridge_test.cc b/cpp/src/arrow/c/bridge_test.cc
index 90fe9d5965..88fc2ed35a 100644
--- a/cpp/src/arrow/c/bridge_test.cc
+++ b/cpp/src/arrow/c/bridge_test.cc
@@ -417,6 +417,10 @@ TEST_F(TestSchemaExport, Map) {
       map(int8(), utf8(), /*keys_sorted=*/true), {"+m", "+s", "c", "u"},
       {"", "entries", "key", "value"},
       {ARROW_FLAG_NULLABLE | ARROW_FLAG_MAP_KEYS_SORTED, 0, 0, 
ARROW_FLAG_NULLABLE});
+
+  // Exports field names and nullability
+  TestNested(map(int8(), field("something", utf8(), false)), {"+m", "+s", "c", 
"u"},
+             {"", "entries", "key", "something"}, {ARROW_FLAG_NULLABLE, 0, 0, 
0});
 }
 
 TEST_F(TestSchemaExport, Union) {
@@ -2802,6 +2806,17 @@ TEST_F(TestSchemaRoundtrip, RegisteredExtension) {
 
 TEST_F(TestSchemaRoundtrip, Map) {
   TestWithTypeFactory([&]() { return map(utf8(), int32()); });
+  TestWithTypeFactory([&]() { return map(utf8(), field("value", int32(), 
false)); });
+  // Field names are brought in line with the spec on import.
+  TestWithTypeFactory(
+      [&]() {
+        return MapType::Make(field("some_entries",
+                                   struct_({field("some_key", utf8(), false),
+                                            field("some_value", int32())}),
+                                   false))
+            .ValueOrDie();
+      },
+      [&]() { return map(utf8(), int32()); });
   TestWithTypeFactory([&]() { return map(list(utf8()), int32()); });
   TestWithTypeFactory([&]() { return list(map(list(utf8()), int32())); });
 }