Read only enough bytes to infer Arrow IPC file schema via stream

[Jefffrey]

Which issue does this PR close?

Closes #6368

Rationale for this change

See issue

What changes are included in this PR?

Adjust behaviour of infer Arrow IPC file schema from stream to infer the schema from the first Schema message in the filer, rather than from the footer of the file, thereby allowing us to only need to read as many bytes as we need to infer schema. Old behaviour was to read entire stream before inferring the schema.

Are these changes tested?

Added new unit tests.

Are there any user-facing changes?

[andygrove]

Shouldn't this error check happen within collect_at_least_n_bytes? I would expect that function to Err if it cannot read at least n bytes.

[Jefffrey]

Good point, fixed to have collect_at_least_n_bytes() do the error checking now

[andygrove]

Related to my previous comment, there is currently no check here that we actually did read at least n bytes.

[Jefffrey]

collect_at_least_n_bytes() should now do the length checking

[tustvold]

FWIW for consistency we might want to do something closer to what we do for parquet where:

• We have an estimate of the size of the footer which we fetch
• We read the actual footer size from the fetched data
• We then fetch any extra data needed
• Once decoded the footer provides information on the schema and where the data blocks are located

This PR instead appears to read the first RecordBatch, whilst I think this should work (provided the file contains data), the more standard approach might be to read the footer.

Edit: I also filed apache/arrow-rs#5021 which outlines some APIs we could add upstream that might help here

[alamb]

Thank you @Jefffrey -- this looks good and well tested to me. Thank you for the initial review @andygrove

cc @jonmmease

[alamb]

What do you think about moving this logic upstream into the arrow-rs reader.

https://github.com/apache/arrow-rs/blob/78735002d99eb0212166924948f95554c4ac2866/arrow-ipc/src/reader.rs#L560

If you agree, I can file an upstream ticket to do so.

[tustvold]

I think I would rather the approach described on apache/arrow-rs#5021, reading the footer is more generally useful, providing information beyond just the schema

[Jefffrey]

What do you think about moving this logic upstream into the arrow-rs reader.

Yes, this logic was essentially ripped from StreamReader and FileReader of arrow-ipc, but adjusted to be made compatible with async stream of bytes. We could move this logic to arrow-ipc, but we need to keep in mind that though we are getting a stream of bytes, this is a stream of bytes in the IPC file format and not the IPC streaming format. So an AsyncStreamReader might not exactly fit our use, whereas an AsyncFileReader could but might be limited if we don't read its footer when attempting to decode the rest of the data.

I think I would rather the approach described on apache/arrow-rs#5021, reading the footer is more generally useful, providing information beyond just the schema

It seems this ticket could be appropriate for that. Just to note, that we can't exactly read the footer in a stream without reverting to the old method of reading the entire stream just to decode the schema.

[tustvold]

reverting to the old method of reading the entire stream just to decode the schema.

The idea would be to do something similar to what we do to read the parquet footer, I provided a few more details on the linked ticket. The trick is to perform ranged reads

[Jefffrey]

Ah apologies, I took a look at the parquet code and I see what you mean now. That indeed would be a better approach, in line with existing behaviour for FileReader 👍

[Jefffrey]

FWIW for consistency we might want to do something closer to what we do for parquet where:

* We have an estimate of the size of the footer which we fetch

* We read the actual footer size from the fetched data

* We then fetch any extra data needed

* Once decoded the footer provides information on the schema and where the data blocks are located

This PR instead appears to read the first RecordBatch, whilst I think this should work (provided the file contains data), the more standard approach might be to read the footer.

Edit: I also filed apache/arrow-rs#5021 which outlines some APIs we could add upstream that might help here

We could read the first chunk of the stream of the file similar to reading the last chunk of parquet and hoping it contains all the necessary data to decode the schema. I chose the current approach as we don't have control over the number of bytes each await brings from the stream, whereas when reading parquet we generally have more control over that I believe.

This method shouldn't read any record batches, it simply reads the first flatbuffer message in the IPC file contents which is expected to be a schema message, per the specification stating that an IPC streaming format should have the schema message come first and the IPC file format is simply an encapsulation of the IPC streaming format with some addiitonal wrapping.

[alamb]

Thanks again @Jefffrey and @tustvold

[alamb]

It appears I accidentally merged this PR without a passing CI run. I have filed a PR to fix: #8037