fix(arrow-json): validate map value nullability - #10475
Merged
Merged
Conversation
Jefffrey
approved these changes
Jul 29, 2026
Contributor
|
thanks @subotac |
Jefffrey
pushed a commit
that referenced
this pull request
Aug 2, 2026
… arrow spec (#10297)" (#10506) # Which issue does this PR close? N/A cc @Jefffrey and @rluvaton # Rationale for this change #10297 changed the default map field names from `keys`/`values` to `key`/`value` to match the Arrow spec. This is a good change to align with the spec, but it is a breaking change: it broke the DataFusion upgrade (apache/datafusion#24030) because data produced elsewhere (e.g. by other Arrow implementations) uses the old `keys`/`values` field names, causing schema mismatches such as: ``` InvalidArgumentError("Incorrect datatype for StructArray field \"metadata\", expected Map(\"entries\": non-null Struct(\"keys\": non-null Utf8, \"values\": Utf8), unsorted) got Map(\"entries\": non-null Struct(\"key\": non-null Utf8, \"value\": Utf8), unsorted)") ``` See discussion on #10297: #10297 (comment) We should hold the field name change for the next breaking release to minimize downstream churn on a minor release, rather than ship it in a minor release. # What changes are included in this PR? - Reverts b963ecf (#10297), restoring the default map field names to `keys`/`values` (plural). - Updates a test added by #10475 after #10297 merged # Are these changes tested? Existing tests. # Are there any user-facing changes? Yes: this reverts the default `MapFieldNames` back to `keys`/`values` (as it was prior to #10297), rather than `key`/`value`. The intent is to reapply #10297 as part of the next breaking release.
MassivePizza
pushed a commit
to massive-com/arrow-rs
that referenced
this pull request
Aug 5, 2026
> AI disclosure: Codex assisted with issue analysis and running local test # Which issue does this PR close? - Closes apache#6391. # Rationale for this change The JSON map decoder constructed map entries with an unchecked `StructArray` constructor. This bypassed child field nullability validation and allowed a null value even when the map value field was non-nullable. # What changes are included in this PR? - Construct map entries with `StructArray::try_new_with_length` so existing schema validation rejects unmasked nulls. - Add a regression test for a null JSON map value with a non-nullable value field. # Are these changes tested? Yes: - `cargo +stable fmt --all -- --check` - `cargo test -p arrow-json --all-features` - `cargo clippy --workspace --all-targets --all-features -- -D warnings` - `git diff --check` # Are there any user-facing changes? Yes. Reading a null JSON map value against a non-nullable value field now returns an error instead of producing an array that violates its schema. There are no public API changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
mapwith non-nullable value schema doesn't error if values are actually null #6391.Rationale for this change
The JSON map decoder constructed map entries with an unchecked
StructArrayconstructor. This bypassed child field nullability validation and allowed a null value even when the map value field was non-nullable.What changes are included in this PR?
StructArray::try_new_with_lengthso existing schema validation rejects unmasked nulls.Are these changes tested?
Yes:
cargo +stable fmt --all -- --checkcargo test -p arrow-json --all-featurescargo clippy --workspace --all-targets --all-features -- -D warningsgit diff --checkAre there any user-facing changes?
Yes. Reading a null JSON map value against a non-nullable value field now returns an error instead of producing an array that violates its schema. There are no public API changes.