Skip to content

fix: Compare metadata of nested child fields in TestingJSONComparison - #993

Merged
paleolimbot merged 1 commit into
apache:mainfrom
JonSnow1807:compare-child-metadata
Oct 26, 2026
Merged

paleolimbot merged 1 commit into
apache:mainfrom
JonSnow1807:compare-child-metadata

Conversation

@JonSnow1807

Copy link
Copy Markdown
Contributor

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.

CompareFieldBase() serializes both fields without metadata and then compares
the metadata of the top-level field only, so a consumer that drops the
metadata of a child of a struct, list, map or union field (e.g., an extension
type on a list item) still passes the comparison.

Recurse through the children of each field (and the children of a dictionary
value type) and compare their metadata at ".children[i].metadata". Empty and
absent metadata are treated as equal there because IPC and the integration
JSON both omit empty metadata.

Fixes apache#973
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.56%. Comparing base (55df060) to head (fa231a5).

Files with missing lines Patch % Lines
src/nanoarrow/testing/testing.cc 33.33% 0 Missing and 12 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #993      +/-   ##
==========================================
- Coverage   78.59%   78.56%   -0.04%     
==========================================
  Files         106      106              
  Lines       17153    17170      +17     
  Branches     2043     2056      +13     
==========================================
+ Hits        13482    13489       +7     
+ Misses       2422     2421       -1     
- Partials     1249     1260      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@paleolimbot paleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@paleolimbot
paleolimbot merged commit 08d23f6 into apache:main Oct 26, 2026
40 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestingJSONComparison ignores metadata on nested child fields

3 participants