You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "etseidl (via GitHub)" <gi...@apache.org> on 2026/09/24 22:49:20 UTC
[PR] parquet: Fix page index loading with suffix prefetch [arrow-rs]
etseidl opened a new pull request, #11208:
URL: https://github.com/apache/arrow-rs/pull/11208
# Which issue does this PR close?
- Closes #11207.
# Rationale for this change
`load_metadata_via_suffix` treated bytes preceding the footer metadata as beginning at file offset zero, despite `MetadataSuffixFetch` not providing the total file size. Page-index loading could consequently slice the wrong bytes or fail with an out-of-bounds range error.
# What changes are included in this PR?
- Stop reusing suffix bytes whose absolute file offset is unknown.
- Fetch page-index data using its actual file range.
- Add a regression test covering page-index loading through `load_via_suffix_and_finish` with a large prefetch hint.
# Are these changes tested?
Yes. The new regression test fails before the fix with:
```text
Corrupted parquet file: index data range (...) exceeds remainder length (...)
```
and passes afterward.
The focused async metadata-reader suite passes:
```text
cargo test -p parquet --lib 'file::metadata::reader::async_tests' --features arrow,async
```
# Are there any user-facing changes?
Page-index loading through `MetadataSuffixFetch` now performs a separate range fetch when the prefetched bytes have no known absolute offset. This prevents incorrect page-index decoding. There are no public API changes.
# AI assistance
I used OpenAI Codex to help investigate the failure, draft the regression test, and implement the fix. I reviewed and understand the resulting changes and take responsibility for them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] parquet: Fix page index loading with suffix prefetch [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on code in PR #11208:
URL: https://github.com/apache/arrow-rs/pull/11208#discussion_r4105436235
##########
parquet/src/file/metadata/reader.rs:
##########
@@ -688,10 +688,9 @@ impl ParquetMetaDataReader {
} else {
let metadata_start = suffix_len - metadata_offset;
let slice = suffix.slice(metadata_start..suffix_len - FOOTER_SIZE);
- Ok((
- self.decode_footer_metadata(slice, file_size, footer)?,
- Some((0, suffix.slice(..metadata_start))),
- ))
+ // `MetadataSuffixFetch` does not expose the file size, so the file offset of
+ // the bytes preceding the metadata is unknown and they cannot be reused safely.
+ Ok((self.decode_footer_metadata(slice, file_size, footer)?, None))
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] parquet: Fix page index loading with suffix prefetch [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #11208:
URL: https://github.com/apache/arrow-rs/pull/11208#discussion_r4102994960
##########
parquet/src/file/metadata/reader.rs:
##########
@@ -688,10 +688,9 @@ impl ParquetMetaDataReader {
} else {
let metadata_start = suffix_len - metadata_offset;
let slice = suffix.slice(metadata_start..suffix_len - FOOTER_SIZE);
- Ok((
- self.decode_footer_metadata(slice, file_size, footer)?,
- Some((0, suffix.slice(..metadata_start))),
- ))
+ // `MetadataSuffixFetch` does not expose the file size, so the file offset of
+ // the bytes preceding the metadata is unknown and they cannot be reused safely.
+ Ok((self.decode_footer_metadata(slice, file_size, footer)?, None))
Review Comment:
this function now always returns `None` for the 2nd tuple arg, perhaps simplify the return type to omit this option now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] parquet: Fix page index loading with suffix prefetch [arrow-rs]
Posted by "etseidl (via GitHub)" <gi...@apache.org>.
etseidl commented on PR #11208:
URL: https://github.com/apache/arrow-rs/pull/11208#issuecomment-5833541194
Thanks @alamb and nice catch @Jefffrey. I'll do the simplification here, shouldn't take too long (famous last words).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] parquet: Fix page index loading with suffix prefetch [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #11208:
URL: https://github.com/apache/arrow-rs/pull/11208
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org