This is an automated email from the ASF dual-hosted git repository.
paleolimbot pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-nanoarrow.git
The following commit(s) were added to refs/heads/main by this push:
new 08d23f64 fix: Compare metadata of nested child fields in
TestingJSONComparison (#993)
08d23f64 is described below
commit 08d23f647c4a81b20bf3af902c93d23401a76b24
Author: Chinmay Shrivastava <[email protected]>
AuthorDate: Sun Oct 25 22:23:14 2026 -0500
fix: Compare metadata of nested child fields in TestingJSONComparison (#993)
### Summary
`TestingJSONComparison::CompareFieldBase` was comparing metadata on the
parent field but not checking metadata on its children.
That meant two nested schemas could compare as equal even when their
child fields had different metadata.
This change makes the comparison recurse into child fields, including
dictionary children, and reuse the existing metadata comparison logic.
It still treats absent and empty metadata as equivalent, and the
existing order-insensitive metadata comparison behavior is preserved.
### Testing
Added a gtest covering metadata differences in list, map, and dictionary
children.
All 291 CTest tests pass.
This is separate from #972, which deals with dictionary value metadata.
Fixes #973.
Co-authored-by: JonSnow1807 <[email protected]>
---
src/nanoarrow/nanoarrow_testing.hpp | 4 ++
src/nanoarrow/testing/testing.cc | 50 ++++++++++++++++
src/nanoarrow/testing/testing_test.cc | 108 ++++++++++++++++++++++++++++++++++
3 files changed, 162 insertions(+)
diff --git a/src/nanoarrow/nanoarrow_testing.hpp
b/src/nanoarrow/nanoarrow_testing.hpp
index 945e19ea..7b6d5f67 100644
--- a/src/nanoarrow/nanoarrow_testing.hpp
+++ b/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);
diff --git a/src/nanoarrow/testing/testing.cc b/src/nanoarrow/testing/testing.cc
index df69cebe..1b481c9d 100644
--- a/src/nanoarrow/testing/testing.cc
+++ b/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;
}
diff --git a/src/nanoarrow/testing/testing_test.cc
b/src/nanoarrow/testing/testing_test.cc
index 8c3cb4c3..1f3ccb7e 100644
--- a/src/nanoarrow/testing/testing_test.cc
+++ b/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;
