💯

Can we also mark ParquetError non exhaustive at the same time (so future additions won't be breaking API changes)?

Like this:

@alamb If add the #[non_exhaustive] to ParquetError, when the downstream matches ParquetError, we have to add the _, https://doc.rust-lang.org/reference/attributes/type_system.html#r-attributes.type-system.non_exhaustive.match
such as

match parquet_error {
  ... => {},
  _ => {} // We have to add the `_`, https://doc.rust-lang.org/reference/attributes/type_system.html#r-attributes.type-system.non_exhaustive.match
}

I'm worried that if we add a new error type into ParquetError, the downstream will be unsensible due to _.

Could you also add some additional comments on how to interpret the argument. Is it the number of bytes that is needed?

Maybe we can return a Range or something more specific about what is needed if it isn't always clear

In an earlier POC I did have two separate errors, one with a range, but we thought that a bit redundant at the time. I suppose we could add an Option<Range<usize>> to NeedMoreData to handle the case of insufficient buffer to read the page indexes. There we know the exact range we need. Right now the needed value is set to the number of bytes read from the tail of the file. If try_parse fails when reading the page indexes, then we really only need the page index range...there's no need to read the footer all over again. But then the error handling becomes more complex...if the range in NeedMoreData is None, then read the required bytes from the tail and call try_parse again; if range is not None, then read the range and call read_page_indexes. Ofc a user is free to ignore the range if they want to keep the error handling simple at the cost of more I/O (but that would likely be negligible since those bytes should still be in buffer cache).

For now I'll try to improve the documentation. Let me know if you want me to add the optional Range to NeedMoreData.

More docs sounds good 👍

👍