fix: use writer types in Skipper for resolved named record types

[ariel-miculas]

Which issue does this PR close?

#9655

Rationale for this change

When a writer-only field references a named Avro type that was previously resolved against a reader schema, parse_type returns the registered reader-resolved type from the shared resolver. This caused two problems:

1. The Skipper built its struct sub-skippers from the reader's field list, which omits writer-only fields. Their bytes were never consumed, leaving the cursor at the wrong position for all subsequent records.

2. Reader fields carry resolution-induced nullability (e.g. a writer plain long matched against a reader ["null", long] gains nullability = Some(NullFirst)). The Skipper read a union-tag byte that was never written, causing "Unexpected EOF" errors.

Fix: store the writer's data type in ResolvedField::ToReader alongside the reader index. The Skipper's Codec::Struct arm now iterates rec.writer_fields and uses the writer type from every entry - both ToReader(_, wdt) and Skip(wdt) - so it always follows the writer's wire format.

What changes are included in this PR?

Are these changes tested?

Yes, added unit tests.

Are there any user-facing changes?

No

[alamb]

FYI @jecsand838

[ariel-miculas]

fixed formatting and rebased onto main;
@jecsand838 can you please take a look?

[jecsand838]

@ariel-miculas

I think the correctness fix here is right.

One improvement I'd recommend is pushing the writer-wire planning fully into codec.rs, instead of carrying full AvroDataTypes down into record.rs and having Skipper::from_avro reconstruct the skip tree there. However this can always be done in a follow-up as well.

[ariel-miculas]

However this can always be done in a follow-up as well.

I'd prefer it this way, since refactoring would take a bit more consideration.

[alamb]

Thank you for this PR @ariel-miculas 🙏

I defer to @jecsand838 's opinion on code structure

Before approving this PR I would would request that we:

1. Try and make the tests easier to understand (see comemnts below)
2. File a ticket that explains what the end user of this crate would see before this code fix (I can't quite tell from the PR description which focuses on what the code does, not the end user visible behavior)

It would also be nice to file a ticket to track @jecsand838 's suggestion for a different structure

[alamb]

these tests seem pretty repetitive and thus it is hard for me to understand what they are supposed to be testing and validate what is different between tehm

Can you please refactor the common functionality into helper functions so the tests themselves are easier to understand and validate ?

[ariel-miculas]

rewrote the tests from a higher level POV and opened #9655

[jecsand838]

Thank you for this PR @ariel-miculas 🙏

I defer to @jecsand838 's opinion on code structure

Before approving this PR I would would request that we:

1. Try and make the tests easier to understand (see comemnts below)
2. File a ticket that explains what the end user of this crate would see before this code fix (I can't quite tell from the PR description which focuses on what the code does, not the end user visible behavior)

It would also be nice to file a ticket to track @jecsand838 's suggestion for a different structure

@alamb I created that tracking ticket: #9651

[ariel-miculas]

These tests fail on main:

$ cargo test -p arrow-avro --lib test_nullable_reader_schema_vs_plain_writer_nested_struct
   Compiling arrow-avro v58.1.0 (/Users/amiculas/work/arrow-rs/arrow-avro)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 2.51s
     Running unittests src/lib.rs (target/debug/deps/arrow_avro-3c0d1f09f75a0c0a)

running 1 test
test reader::test::test_nullable_reader_schema_vs_plain_writer_nested_struct ... FAILED

failures:

---- reader::test::test_nullable_reader_schema_vs_plain_writer_nested_struct stdout ----

thread 'reader::test::test_nullable_reader_schema_vs_plain_writer_nested_struct' (11467926) panicked at arrow-avro/src/reader/mod.rs:9670:14:
reading should succeed: AvroError("Parser error: bad varint")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    reader::test::test_nullable_reader_schema_vs_plain_writer_nested_struct

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 392 filtered out; finished in 0.01s

error: test failed, to rerun pass `-p arrow-avro --lib`
~/work/arrow-rs main* ⇡
$ cargo test -p arrow-avro --lib test_skip_array_of_structs_uses_writer_schema_not_resolved
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.09s
     Running unittests src/lib.rs (target/debug/deps/arrow_avro-3c0d1f09f75a0c0a)

running 1 test
test reader::test::test_skip_array_of_structs_uses_writer_schema_not_resolved ... FAILED

failures:

---- reader::test::test_skip_array_of_structs_uses_writer_schema_not_resolved stdout ----

thread 'reader::test::test_skip_array_of_structs_uses_writer_schema_not_resolved' (11468665) panicked at arrow-avro/src/reader/mod.rs:9793:14:
Skipper must consume all events bytes using writer field types: AvroError("Parser error: bad varint")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    reader::test::test_skip_array_of_structs_uses_writer_schema_not_resolved

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 392 filtered out; finished in 0.00s

error: test failed, to rerun pass `-p arrow-avro --lib`
~/work/arrow-rs main* ⇡
$ cargo test -p arrow-avro --lib test_skipper_consumes_writer_only_struct_fields
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.08s
     Running unittests src/lib.rs (target/debug/deps/arrow_avro-3c0d1f09f75a0c0a)

running 1 test
test reader::test::test_skipper_consumes_writer_only_struct_fields has been running for over 60 seconds
^C

with the last one stuck in an infinite loop

[ariel-miculas]

@alamb please let me know if the new tests are good

[alamb]

LGTM -- thanks @ariel-miculas and @jecsand838