Arrow-Avro: Resolve named field discrepancies #8546
Merged
Conversation
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
github-actions
bot
added
arrow
jecsand838
force-pushed
the
resolve_named_field
branch
from
cdc38d4 to
72c2904
Compare
Contributor
|
@mbrobbel @alamb If there's anyway this PR and #8550 can get into the 57.0.0 release that would be huge. After this there's just one last PR to add the remaining round trip tests. These two PRs will complete the remaining functionality though. CC: @nathaniel-d-ef |
mbrobbel
reviewed
Oct 6, 2025
jecsand838
force-pushed
the
resolve_named_field
branch
from
f6bf96c to
6122ef9
Compare
jecsand838
force-pushed
the
resolve_named_field
branch
from
a877ba5 to
45e816e
Compare
mbrobbel
approved these changes
Oct 6, 2025
Member
mbrobbel
left a comment
mbrobbel
left a comment
There was a problem hiding this comment.
Thanks @nathaniel-d-ef and @jecsand838
Contributor
|
🚀 |
mbrobbel
added a commit
that referenced
this pull request
Oct 8, 2025
# Which issue does this PR close? Relates to: #8348 #4886 # Rationale for this change This PR completes the efforts of @jecsand838, adding dense union support to the encoder side of the crate, along with four other minor extensions of existing time-related encoding. Note: currently this PR is stacked behind #8546. Once that's merged this will be updated and will not include those changes. # What changes are included in this PR? - Dense union support for the writer - Tests # Are these changes tested? - A full round-trip test, reading in an existing union avro file and asserting that the output matches expectations - Unit tests covering new encoders. # Are there any user-facing changes? Crate not yet public --------- Co-authored-by: Connor Sanders <[email protected]> Co-authored-by: Connor Sanders <[email protected]> Co-authored-by: Matthijs Brobbel <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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?
Related to: #4886 (“Add Avro Support”)
Rationale for this change
Prior to this PR, the crate lacked sufficient support for named types.
This PR introduces fixes to the Avro reader and writer to ensure correct and robust roundtrip serialization of complex union types. The core issue was that the previous implementation failed to properly distinguish between logically distinct types within a union if they shared the same physical representation. This fundamental flaw led to valid union schemas/data being flagged as invalid, the loss of specific names of named type branches (e.g., "Fx8" becoming "fixed").
This PR makes a change whereby name and namespace data is registered and retrieved from metadata, which ensures that complex Avro unions can now be reliably read, converted to Arrow, and written back to Avro without validation errors or loss of type information.
This behavior will be further validated in a follow-up PR to add support for writing dense unions.
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?
Crate not yet public