ahlag commented on issue #11736:
URL: https://github.com/apache/arrow-rs/issues/11736#issuecomment-6283994583
I'd like to take this. I reproduced it on main: over every split position of
the 81-byte stream in the report, 37 decode correctly, 35 give wrong rows and 8
fail with `bad varint`.
Before writing the PR, I'd like to check the approach, since it touches
every field decoder:
1. **Roll back on incomplete data.** `RecordDecoder::decode` remembers
`row_count` before each record. If a field returns EOF / need-more-data, it
truncates every field decoder back to that many rows before returning the
error. That needs a `Decoder::truncate(rows)`: `Vec::truncate` for the
primitive arms, offsets plus child values for String/Binary/List/Map,
per-branch counts for unions, and `pending_nulls` / validity for `Nullable`.
The happy path costs nothing.
2. **Treat a truncated varint as EOF.** Make `AvroCursor::read_vlq` return
`EOF` instead of `ParseError("bad varint")` when the buffer ends inside a
varint, so `is_incomplete_data` treats it as "need more bytes". This has to
land with (1). On its own it would turn those 8 errors into silently wrong rows.
The alternative is to run the existing `Skipper` over the body first and
only decode once the whole record is present. That is much less code, but every
framed record would be parsed twice, so I'd go with (1).
A few arms have no cheap truncate today: the `Decimal*` and `Duration`
builders (`PrimitiveBuilder` has no `truncate`) and `RunEndEncoded`. I'd either
switch those to `Vec`-backed storage like the other arms, or rebuild them on
rollback.
The test would decode the report's list, map and `{id, tags}` shapes split
at every position and compare each result with the one-shot decode.
Does this direction sound right? cc @Jefffrey
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
