Follow-up Improvements to Avro union handling #8385
Conversation
…a constraints, and improve error reporting. Add tests for missing union branch names, named type signatures, and codec validation. Optimize union variant resolution logic and simplify schema traversal for complex types.
github-actions
bot
added
arrow
|
@scovich Here's the follow-up that incorporates your recommendations. Let me know what you think. |
| } | ||
| } | ||
| } | ||
| let (reader_index, promotion) = direct.or(promo).ok_or_else(|| { |
There was a problem hiding this comment.
Can delete direct now (L1363 above)?
arrow-avro/src/codec.rs
Outdated
| fn mk_primitive<'a>(pt: PrimitiveType) -> Schema<'a> { | ||
| Schema::TypeName(TypeName::Primitive(pt)) |
There was a problem hiding this comment.
Interesting... 'a doesn't "attach" to any input arg? Not quite sure how that works in rust...
There was a problem hiding this comment.
Does this work, out of curiosity?
| fn mk_primitive<'a>(pt: PrimitiveType) -> Schema<'a> { | |
| Schema::TypeName(TypeName::Primitive(pt)) | |
| fn mk_primitive(pt: PrimitiveType) -> Schema<'_> { | |
| Schema::TypeName(TypeName::Primitive(pt)) |
There was a problem hiding this comment.
Nope... might be a case where 'static actually makes sense?
There was a problem hiding this comment.
I hit these same issues. Dealing with mk_primitive is initially what led me to make the others static.
There was a problem hiding this comment.
I'll just change mk_primitive back to being static.
Co-authored-by: Ryan Johnson <[email protected]>
@scovich Ty for the quick review. I just pushed up some improvements based on your comments. Let me know what you think. |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thank you @jecsand838 and @scovich
3 participants
Which issue does this PR close?
This work continues arrow-avro schema resolution support and aligns behavior with the Avro spec.
Rationale for this change
@scovich left a really solid review on #8348 that wasn't completed until after the PR was merged in. This PR is for addressing the suggestions and improving the code.
What changes are included in this PR?
codec.rsschema.rsincluding spec compliant named type errors.Are these changes tested?
codec.rsand all existing tests are passing without changes.schema.rsto cover the spec compliant named type changes.Are there any user-facing changes?
N/A