fix(arrow/array): optional struct array with required field#359
Merged
zeroshade merged 1 commit intoapache:mainfrom Apr 25, 2025
Merged
fix(arrow/array): optional struct array with required field#359zeroshade merged 1 commit intoapache:mainfrom
zeroshade merged 1 commit intoapache:mainfrom
Conversation
4fe4519 to
133f434
Compare
lidavidm
approved these changes
Apr 22, 2025
Member
lidavidm
left a comment
There was a problem hiding this comment.
I feel like Arrow should either sit down and specify the semantics of a nullable field, or else it should explicitly declare that validation is up to the application
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
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.
Rationale for this change
apache/iceberg-go#398 discovered that the current
NewStructArrayWithFieldsfails if any child is marked as non-nullable but has nulls (as would be the case in an optional struct array full of nulls with a required field). So we need to allow constructing such an array.What changes are included in this PR?
A new function is created,
NewStructArrayWithFieldsAndNullswhich takes in the top level struct null bitmap, the number of nulls, offset columns and list of fields.Are these changes tested?
Yes, a unit test was created for it.
Are there any user-facing changes?
The above case that would error, now will no longer error.