-
Notifications
You must be signed in to change notification settings - Fork 906
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ArrowReaderMetadata API makes it too easy to (accidentally) make an additional object store request #6476
Comments
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 |
Thank you, get it. |
@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. |
Thanks @etseidl -- that would be great |
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
toArrowReaderMetadata::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
haswith_page_index
specified but the provided metadata doesn't (yet) have the page index, it will load it againThis is a common source of confusion / bugs: when someone supplies the
ParquetMetaData
to theArrowReaderMetadata
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 intoParquetMetaData
Additional context
The text was updated successfully, but these errors were encountered: