Add tests and fixes for schema resolution bug #9237
+191
−17
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
…ay items and unions.
github-actions
bot
added
arrow
mzabaluev
reviewed
Jan 21, 2026
Contributor
mzabaluev
left a comment
mzabaluev
left a comment
There was a problem hiding this comment.
It works! The changes look good, I have few comments on code reuse.
arrow-avro/src/codec.rs
Outdated
Comment on lines
1532
to
1535
| let null_position = reader_variants | ||
| .iter() | ||
| .position(|x| x == &Schema::TypeName(TypeName::Primitive(PrimitiveType::Null))); | ||
| if let (2, Some(null_idx)) = (reader_variants.len(), null_position) { |
Contributor
There was a problem hiding this comment.
Would it be good to reuse the nullable_union_variants helper here?
| let manifest_dir = env!("CARGO_MANIFEST_DIR"); | ||
| let gz_path = format!("{manifest_dir}/test/data/bad-varint-bug.avro.gz"); | ||
| let gz_file = File::open(&gz_path).expect("test file should exist"); | ||
| let mut decoder = GzDecoder::new(gz_file); |
Contributor
There was a problem hiding this comment.
I actually had the file unpacked in my test suite branch, it's quite small. I only gzipped it to be able to post to GitHub. But since there's already a dependency on flate2, might as well use it.
arrow-avro/src/codec.rs
Outdated
| let non_null_idx = 1 - null_idx; | ||
| let non_null_branch = &reader_variants[non_null_idx]; | ||
| let mut dt = | ||
| self.make_data_type(writer_non_union, Some(non_null_branch), namespace)?; |
Contributor
There was a problem hiding this comment.
I'd call resolve_type directly here because that's what it always amounts to.
Co-authored-by: Mikhail Zabaluev <mikhail.zabaluev@flarion.io>
Contributor
Author
|
@mzabaluev Thank you for the review! I just pushed up a commit that addresses your comments. Let me know what you think when you get a chance. |
jecsand838
force-pushed
the
arrow-avro-bug
branch
from
6f13808 to
b2f475d
Compare
jecsand838
force-pushed
the
arrow-avro-bug
branch
from
b2f475d to
919bdf4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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?
ParseError("bad varint")on reading array field with nullable elements in the reader schema #9231.Rationale for this change
Avro schema resolution allows a reader schema to represent “nullable” values using a two-branch union (
["null", T]or[T, "null"]) while still reading data written with the non-union schemaT(i.e. without union discriminants in the encoded data).In
arrow-avro, resolving a non-union writer type against a reader union (notably for array/list item schemas likeitems: ["null", "int"]) could incorrectly treat the encoded stream as a union and attempt to decode a union discriminant. This would misalign decoding and could surface asParseError("bad varint")for certain files (see #9231).What changes are included in this PR?
Fix schema resolution when the writer schema is non-union and the reader schema is a union:
nullto be treated as “nullable” (capturing whethernullis first or second), and resolve against the non-null branch.Add regression coverage for the bug and the fixed behavior:
test_resolve_array_writer_nonunion_items_reader_nullable_items(schema resolution / codec)test_array_decoding_writer_nonunion_items_reader_nullable_items(record decoding; ensures correct byte consumption and decoded values)test_bad_varint_bug_nullable_array_items(end-to-end reader regression using a small Avro fixture)Add a small compressed Avro fixture under
arrow-avro/test/data/bad-varint-bug.avro.gzused by the regression test.Are these changes tested?
Yes. This PR adds targeted unit/integration tests that reproduce the prior failure mode and validate correct schema resolution and decoding for nullable-union array items.
Are there any user-facing changes?
Yes (bug fix): reading Avro files with arrays whose element type is represented as a nullable union in the reader schema (e.g.
items: ["null", "int"]) now succeeds instead of failing withParseError("bad varint"). No public API changes are intended.