ArrowReaderMetadata API makes it too easy to (accidentally) make an additional object store request

[alamb]

Is your feature request related to a problem or challenge? Please describe what you are trying to do.

ArrowReaderMetadata to read parquet files, and one major usecase is to supply pre-parsed metadata (to avoid a second object store request on read) by providing the ParquetMetaData to ArrowReaderMetadata::try_new

However, the way the API is currently setup it is easy to supply the ParquetMetaData but the reader will STILL make 2 object store requests.

This happens if the ArrowReaderOptions has with_page_index specified but the provided metadata doesn't (yet) have the page index, it will load it again

This is a common source of confusion / bugs: when someone supplies the ParquetMetaData to the ArrowReaderMetadata they are very often trying to avoid a second object store request, but as it often turns out the second fetch happens anyways to read the page index (thus obviating the attempt at optimization)

This is (in a roundabout way) what is happening to @progval in apache/datafusion#12593 and it took me a while to debug what was happening while working on the advanced_parquet_index.rs in DataFusion

Describe the solution you'd like
I would like the API to be harder to misuse.

Describe alternatives you've considered
For example, maybe we could make ArrowReaderMetadata error if it was supplied with ParquetMetaData that did not have the page indexes,

for example, we could add a ArrowReaderOptions::error_if_need_metadata or something that would change the automatic fetch/load behavior into an error if the reader needs the page index, and the file has a page index, but it isn't loaded yet into ParquetMetaData

Additional context

[doki23]

I propose another approach: we could cache the page index upon its initial load ( make user provided metadata mutable), as the current API seems satisfactory to me.

[alamb]

I propose another approach: we could cache the page index upon its initial load ( make user provided metadata mutable), as the current API seems satisfactory to me.

I am not quite sure what you mean by this -- depending on how you setup the initial load, the page index will be read. The issue I was trying to describe is if you don't read the page index in the initial load, a subsequent opening of the file even if you supply ParquetMetaData, will force another fetch of metadata (page index) prior to reading the data

[doki23]

I propose another approach: we could cache the page index upon its initial load ( make user provided metadata mutable), as the current API seems satisfactory to me.

I am not quite sure what you mean by this -- depending on how you setup the initial load, the page index will be read. The issue I was trying to describe is if you don't read the page index in the initial load, a subsequent opening of the file even if you supply ParquetMetaData, will force another fetch of metadata (page index) prior to reading the data

Thank you, get it.

[etseidl]

@adamreeve recently jogged my memory of this issue (#7342 (comment)). I think the recent changes to the API (#6637, #7334, #7342) make fixing this finally possible. I'll take a stab at it this week.

[alamb]

Thanks @etseidl -- that would be great