fix: Compare metadata of nested child fields in TestingJSONComparison

src/nanoarrow/nanoarrow_testing.hpp

@@ -278,6 +278,10 @@ class NANOARROW_DLL TestingJSONComparison {
   ArrowErrorCode CompareMetadata(const char* actual, const char* expected,
                                  ArrowError* error, const std::string& path = "");
 
+  ArrowErrorCode CompareChildrenMetadata(const ArrowSchema* actual,
+                                         const ArrowSchema* expected, ArrowError* error,
+                                         const std::string& path = "");
+
   ArrowErrorCode MetadataEqualKeyValue(const char* actual, const char* expected,
                                        bool* out, ArrowError* error);
 

src/nanoarrow/testing/testing.cc

@@ -2778,6 +2778,56 @@ ArrowErrorCode TestingJSONComparison::CompareFieldBase(ArrowSchema* actual,
 
   NANOARROW_RETURN_NOT_OK(CompareMetadata(actual->metadata, expected->metadata, error,
                                           path + std::string(".metadata")));
+  NANOARROW_RETURN_NOT_OK(CompareChildrenMetadata(actual, expected, error, path));
+  return NANOARROW_OK;
+}
+
+namespace {
+
+// Empty metadata is equivalent to no metadata (e.g., both are omitted when written to
+// IPC or to the integration JSON)
+ArrowErrorCode MetadataOrNullIfEmpty(const char* metadata, const char** out) {
+  ArrowMetadataReader reader;
+  NANOARROW_RETURN_NOT_OK(ArrowMetadataReaderInit(&reader, metadata));
+  *out = reader.remaining_keys > 0 ? metadata : nullptr;
+  return NANOARROW_OK;
+}
+
+}  // namespace
+
+ArrowErrorCode TestingJSONComparison::CompareChildrenMetadata(const ArrowSchema* actual,
+                                                              const ArrowSchema* expected,
+                                                              ArrowError* error,
+                                                              const std::string& path) {
+  // The serialized field (which does not include metadata) was already compared by
+  // the caller and records any difference in structure (e.g., the number of children
+  // or the presence of a dictionary), so only the metadata of children that exist in
+  // both fields is compared here.
+  if (actual->n_children == expected->n_children) {
+    for (int64_t i = 0; i < expected->n_children; i++) {
+      std::string child_path = path + ".children[" + std::to_string(i) + "]";
+
+      const char* actual_metadata;
+      const char* expected_metadata;
+      NANOARROW_RETURN_NOT_OK_WITH_ERROR(
+          MetadataOrNullIfEmpty(actual->children[i]->metadata, &actual_metadata), error);
+      NANOARROW_RETURN_NOT_OK_WITH_ERROR(
+          MetadataOrNullIfEmpty(expected->children[i]->metadata, &expected_metadata),
+          error);
+      NANOARROW_RETURN_NOT_OK(CompareMetadata(actual_metadata, expected_metadata, error,
+                                              child_path + ".metadata"));
+
+      NANOARROW_RETURN_NOT_OK(CompareChildrenMetadata(
+          actual->children[i], expected->children[i], error, child_path));
+    }
+  }
+
+  // The children of a dictionary-encoded field are those of the dictionary value type
+  if (actual->dictionary != nullptr && expected->dictionary != nullptr) {
+    NANOARROW_RETURN_NOT_OK(CompareChildrenMetadata(
+        actual->dictionary, expected->dictionary, error, path + ".dictionary"));
+  }
+
   return NANOARROW_OK;
 }
 

src/nanoarrow/testing/testing_test.cc

@@ -1655,6 +1655,114 @@ TEST(NanoarrowTestingTest, NanoarrowTestingTestMetadataComparison) {
   ASSERT_EQ(comparison.CompareSchema(actual.get(), expected.get()), ENOTSUP);
 }
 
+TEST(NanoarrowTestingTest, NanoarrowTestingTestNestedMetadataComparison) {
+  nanoarrow::UniqueSchema actual;
+  nanoarrow::UniqueSchema expected;
+  nanoarrow::UniqueBuffer buf;
+
+  ASSERT_EQ(ArrowMetadataBuilderInit(buf.get(), nullptr), NANOARROW_OK);
+  ASSERT_EQ(ArrowMetadataBuilderAppend(buf.get(), "key1"_asv, "value1"_asv),
+            NANOARROW_OK);
+  const char* metadata = reinterpret_cast<char*>(buf->data);
+
+  // Start with two identical schemas whose list item has metadata
+  ArrowSchemaInit(actual.get());
+  ASSERT_EQ(ArrowSchemaSetTypeStruct(actual.get(), 1), NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaSetType(actual->children[0], NANOARROW_TYPE_LIST), NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaSetType(actual->children[0]->children[0], NANOARROW_TYPE_INT32),
+            NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaSetMetadata(actual->children[0]->children[0], metadata),
+            NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaDeepCopy(actual.get(), expected.get()), NANOARROW_OK);
+  AssertSchemasCompareEqual(actual.get(), expected.get());
+
+  // With the metadata of the list item dropped from the actual schema
+  ASSERT_EQ(ArrowSchemaSetMetadata(actual->children[0]->children[0], nullptr),
+            NANOARROW_OK);
+  AssertSchemasCompareUnequal(actual.get(), expected.get(), /*num_differences*/ 1,
+                              /*differences*/
+                              "Path: .children[0].children[0].metadata"
+                              R"(
+- null
++ [{"key": "key1", "value": "value1"}]
+
+)");
+
+  // ...also using the comparison that does *not* consider ordering
+  AssertSchemasCompareUnequal(actual.get(), expected.get(), /*num_differences*/ 1,
+                              /*differences*/
+                              "Path: .children[0].children[0].metadata"
+                              R"(
+- null
++ [{"key": "key1", "value": "value1"}]
+
+)",
+                              [](TestingJSONComparison& comparison) {
+                                comparison.set_compare_metadata_order(false);
+                              });
+
+  // Empty metadata on a child is equivalent to no metadata (IPC and the integration
+  // JSON both omit empty metadata)
+  ASSERT_EQ(ArrowSchemaSetMetadata(actual->children[0]->children[0], "\0\0\0\0"),
+            NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaSetMetadata(expected->children[0]->children[0], nullptr),
+            NANOARROW_OK);
+  AssertSchemasCompareEqual(actual.get(), expected.get());
+
+  // Metadata on the value child of a map is compared (the map's canonical names
+  // are forced before comparison, which must not affect the metadata)
+  actual.reset();
+  expected.reset();
+  ArrowSchemaInit(actual.get());
+  ASSERT_EQ(ArrowSchemaSetTypeStruct(actual.get(), 1), NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaSetType(actual->children[0], NANOARROW_TYPE_MAP), NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaSetType(actual->children[0]->children[0]->children[0],
+                               NANOARROW_TYPE_STRING),
+            NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaSetType(actual->children[0]->children[0]->children[1],
+                               NANOARROW_TYPE_INT32),
+            NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaDeepCopy(actual.get(), expected.get()), NANOARROW_OK);
+  ASSERT_EQ(
+      ArrowSchemaSetMetadata(expected->children[0]->children[0]->children[1], metadata),
+      NANOARROW_OK);
+  AssertSchemasCompareUnequal(actual.get(), expected.get(), /*num_differences*/ 1,
+                              /*differences*/
+                              "Path: .children[0].children[0].children[1].metadata"
+                              R"(
+- null
++ [{"key": "key1", "value": "value1"}]
+
+)");
+
+  // Metadata on a child of a dictionary value type is compared
+  actual.reset();
+  expected.reset();
+  ArrowSchemaInit(actual.get());
+  ASSERT_EQ(ArrowSchemaSetTypeStruct(actual.get(), 1), NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaSetType(actual->children[0], NANOARROW_TYPE_INT32), NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaAllocateDictionary(actual->children[0]), NANOARROW_OK);
+  ArrowSchemaInit(actual->children[0]->dictionary);
+  ASSERT_EQ(ArrowSchemaSetTypeStruct(actual->children[0]->dictionary, 1), NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaSetType(actual->children[0]->dictionary->children[0],
+                               NANOARROW_TYPE_INT32),
+            NANOARROW_OK);
+  ASSERT_EQ(ArrowSchemaDeepCopy(actual.get(), expected.get()), NANOARROW_OK);
+  AssertSchemasCompareEqual(actual.get(), expected.get());
+
+  ASSERT_EQ(
+      ArrowSchemaSetMetadata(expected->children[0]->dictionary->children[0], metadata),
+      NANOARROW_OK);
+  AssertSchemasCompareUnequal(actual.get(), expected.get(), /*num_differences*/ 1,
+                              /*differences*/
+                              "Path: .children[0].dictionary.children[0].metadata"
+                              R"(
+- null
++ [{"key": "key1", "value": "value1"}]
+
+)");
+}
+
 TEST(NanoarrowTestingTest, NanoarrowTestingTestArrayComparison) {
   nanoarrow::UniqueSchema schema;
   nanoarrow::UniqueArray actual;