Repository navigation
[59_maintenance] fix(parquet): skip miniblocks wider than 64 values instead of erroring - #11754
Merged
Merged
Conversation
apache#11021) # Which issue does this PR close? - Closes apache#11018. # Rationale for this change `DeltaBitPackDecoder::skip` sized its scratch buffer with a `match` on `values_per_mini_block` that only accepted 32 and 64, and returned `cannot skip miniblock of size N` for anything else. The spec only requires a multiple of 32, and Databricks Photon 0.2 writes 256, so valid files from that engine fail to read as soon as a `RowSelector::skip` is involved. `get` never had the restriction because it chunks against the caller's buffer. The scratch buffer only exists to walk `last_value` forward through the deltas. `BitReader::get_batch` carries its own `bit_offset` and `byte_offset`, so reading a 256 value miniblock as four calls of 64 leaves the reader exactly where one call of 256 would. That means the buffer can stay small and the miniblock can be consumed a chunk at a time, which was the concern that closed apache#9793 without a fix. # What changes are included in this PR? - Size `skip_buffer` at `values_per_mini_block.min(64)` and loop over the miniblock in chunks of it, instead of rejecting sizes other than 32 and 64. - Flip `non_standard_delta_blocks` in `bad_data.rs` from asserting the error to asserting the skip lands on the right rows. That test already carried a commented-out `skip should succeed` and a TODO for this change. For 32 and 64 the buffer is the same length it was before and the new loop runs exactly once, so the path this crate's own writer produces allocates and reads what it did before. # Are these changes tested? - New `test_delta_bit_packed_skip_wide_miniblocks` builds a hand-written page with `block_size = 1024` over 4 miniblocks, so 256 values each. It fails on `main` with `cannot skip miniblock of size 256` and passes here. It checks the values after a skip that stops mid-miniblock, and after one that crosses into a `bit_width = 0` miniblock, since both depend on `last_value` surviving the chunk boundary. - `non_standard_delta_blocks` now reads `bigdelta.parquet` twice, once with the row selection and once without, and compares the 5 selected rows against `all.slice(1000, 5)`. Asserting only `num_rows() == 5` would pass on wrong values. - `cargo test -p parquet --all-features`: 1426 lib tests and 456 integration tests pass, 0 failures. - `cargo clippy -p parquet --all-features --all-targets`: clean. - No benchmark numbers here because the change doesn't claim a speedup. The argument that the common path is untouched is the buffer length and loop count above, not a measurement. # Are there any user-facing changes? Reading a page with miniblocks wider than 64 values under a row selection now returns data instead of an error. No public API changes. There's also an existing overflow bug in `skip`. The `bit_width == 0` branch of `skip` computes `min_delta * n` in `i64` and then converts to `T::T`, so a valid INT32 progression can fail the conversion even though every decoded value fits. A page with `first_value = -2000000000`, `min_delta = 10000000` and zero-width miniblocks reads fine through `get`, giving 560000000 at index 256, but `skip(257)` returns `delta*n overflow in skip`. That's independent of miniblock width and predates this change, so I've left it for a separate issue and fix. # AI usage Claude wrote the change and the tests, and Codex reviewed the diff adversarially before it was pushed. I checked every finding myself: the split-read equivalence against `BitReader::get_batch`, the truncated-page error path, and the overflow above, which I reproduced before writing it down. The PR description is AI-assisted. --------- Co-authored-by: Ed Seidl <etseidl@users.noreply.github.com>
Contributor
|
Thank you @Jefffrey and @Rich-T-kid |
antonireus
added a commit
to exasol-labs/lakehouse-engine-rs
that referenced
this pull request
Oct 26, 2026
parquet 59.3.2 backports apache/arrow-rs#11754, so a filter that skips rows of a DELTA_BINARY_PACKED column with 256-value miniblocks (written by Databricks Photon) no longer fails with `cannot skip miniblock of size 256`. Closes #504 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
antonireus
added a commit
to exasol-labs/lakehouse-engine-rs
that referenced
this pull request
Oct 26, 2026
parquet 59.3.2 backports apache/arrow-rs#11754, so a filter that skips rows of a DELTA_BINARY_PACKED column with 256-value miniblocks (written by Databricks Photon) no longer fails with `cannot skip miniblock of size 256`. Closes #504 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
59.3.2(October 2026) #11664Cherry-pick of 44141e6