feat: two-stage builder API for async Avro reader#9462
feat: two-stage builder API for async Avro reader#9462mzabaluev wants to merge 1 commit intoapache:mainfrom
Conversation
Expose the read_header method in reader::async_reader::ReaderBuilder, returning another builder typestate that exposes the writer schema as it was read from the file header.
github-actions
bot
added
arrow
|
|
||
| impl<R: AsyncFileReader> ReaderBuilder<R> { | ||
| async fn read_header(&mut self) -> Result<(Header, u64), AvroError> { | ||
| impl<R> ReaderBuilder<R> |
There was a problem hiding this comment.
I wonder, do we want to allow maybe a with_header function as well? that will accept a user's header directly?
There was a problem hiding this comment.
Not the typical usecase, but makes it more flexible
There was a problem hiding this comment.
What would be the behavior with this method? Skip reading the header from the file, and start decoding from...?
There was a problem hiding this comment.
Presumable the range?
The behaviour would be the exact same, since the header ends with the magic I believe? and we start the actual decoding from the first magic we encounter
There was a problem hiding this comment.
So the use case would be, the application parses the header once (or just supplies their own), and then passes it to read ranges in the file on the object store, assuming the header stays the same?
There was a problem hiding this comment.
Header is not currently public, but this could be just an oversight. Its interface looks public-ready.
There was a problem hiding this comment.
So the use case would be, the application parses the header once (or just supplies their own), and then passes it to read ranges in the file on the object store, assuming the header stays the same?
At the worst case (range is 0-something), we scan the header bytes very fast until we find the magic, no decoding needed, then we start scanning normally.
Best case is range is middleOfFile-something, and we don't need to do the first call to read the header at all since the user provided it. we just scan until the first magic and party on
There was a problem hiding this comment.
Headeris not currently public, but this could be just an oversight. Its interface looks public-ready.
I also think so, but maybe it's better to do this in a separate PR, making this public has a tendency to bite back 😅
There was a problem hiding this comment.
Presumable the range?
What if the range is not given?
The behaviour would be the exact same, since the header ends with the magic I believe?
The current behavior uses the discovered length of the header as it was parsed from the file.
If the application supplies its own, the with_header method should also give the length, i.e. the offset past the header to start parsing the data from. Alternatively, we could just scan for the magic from the start of the file (unless the range option directs otherwise), but I'm not sure this is bulletproof.
There was a problem hiding this comment.
if range is not given it is 0..EOF
in which case, as I said - we scan the bytes quickly for the magic(which was provided in the header by the user), no decoding happens, then we start decoding normally.
2 participants
Which issue does this PR close?
What changes are included in this PR?
Expose the
read_headermethod inreader::async_reader::ReaderBuilder, returning another builder typestate that exposes the writer schema as it was read from the file header.Are these changes tested?
Tests and doc tests to be added for the new API, showing possible use.
Are there any user-facing changes?
The new API augments the existing ReaderBuilder in a backward-compatible way.