Repository navigation
test(parquet): show push decoder needs a whole row group before decoding a batch - #11218
Merged
Merged
Conversation
…ing a batch Add a test documenting that ParquetPushDecoder requests all pages for all projected columns when it starts a row group, and produces no output until every requested range is present, even when the pages already pushed are sufficient to decode the next batch. Also shows that adjacent pushed ranges are not coalesced to satisfy a larger requested range. Related to apache#6946 Co-Authored-By: Claude Fable 5.1 <[email protected]>
alamb
marked this pull request as ready for review
September 25, 2026 17:17
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Sep 26, 2026
* `test_decoder_first_pages_only_batch_granularity`, the counterpart of `test_decoder_first_pages_only` from apache#11218. That test shows that the default `FetchGranularity::RowGroup` does not decode a batch until the whole row group is pushed. With `FetchGranularity::Batch`, pushing the first page of each column returns the first batch. * `incremental_tests`: both granularities give the same batches for batch sizes around page and row-group sizes, projections with list and struct columns, page-skipping row selections with each selection policy, offset and limit, files without an offset index, predicate chains with and without the predicate cache (including cache misses), and 300 random combinations of all of these. Other tests check that requests serve one batch, that dictionary pages are requested once, that no range is requested twice, that bytes pushed ahead are released as the readers pass them, that the row-group boundary is visible after the last batch, and `into_builder` and `try_next_reader` interplay. Co-Authored-By: Claude Opus 5.5 <[email protected]>
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Sep 26, 2026
* `test_decoder_first_page_only_does_not_decode` from apache#11218, which shows that the row-group mode does not decode a batch until the whole row group is pushed, and a counterpart for `FetchGranularity::Batch` where pushing the first page of each column returns the first batch. * `incremental_tests`: both granularities give the same batches for batch sizes around page and row-group sizes, projections with list and struct columns, page-skipping row selections with each selection policy, offset and limit, files without an offset index, predicate chains with and without the predicate cache (including cache misses), and 300 random combinations of all of these. Other tests check that requests serve one batch, that dictionary pages are requested once, that no range is requested twice, that bytes pushed ahead are released as the readers pass them, that the row-group boundary is visible after the last batch, and `into_builder` and `try_next_reader` interplay. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This was referenced Sep 27, 2026
Contributor
Author
Contributor
|
Sure, I added it to my queue over the weekend, but I'll bump the priority. |
Contributor
Author
Thank you for the review. I am aware I owe you one too (but I have to run now so I didn't make it today). Will do tomorrow |
Contributor
No worries, I'm in no hurry. |
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Sep 28, 2026
* `test_decoder_first_pages_only_batch_granularity`, the counterpart of `test_decoder_first_pages_only` from apache#11218. That test shows that the default `FetchGranularity::RowGroup` does not decode a batch until the whole row group is pushed. With `FetchGranularity::Batch`, pushing the first page of each column returns the first batch. * `incremental_tests`: both granularities give the same batches for batch sizes around page and row-group sizes, projections with list and struct columns, page-skipping row selections with each selection policy, offset and limit, files without an offset index, predicate chains with and without the predicate cache (including cache misses), and 300 random combinations of all of these. Other tests check that requests serve one batch, that dictionary pages are requested once, that no range is requested twice, that bytes pushed ahead are released as the readers pass them, that the row-group boundary is visible after the last batch, and `into_builder` and `try_next_reader` interplay. Co-Authored-By: Claude Opus 5.5 <[email protected]>
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Sep 28, 2026
test_decoder_first_pages_only_batch_granularity is the counterpart of the test from apache#11218. incremental_tests.rs checks that both granularities give the same batches (targeted and randomized), and checks the requests, the row-group boundary and the bytes held. Co-Authored-By: Claude Opus 5.5 <[email protected]>
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Sep 28, 2026
test_decoder_first_pages_only_batch_granularity is the counterpart of the test from apache#11218. incremental_tests.rs checks that both granularities give the same batches (targeted and randomized), and checks the requests, the row-group boundary and the bytes held. Co-Authored-By: Claude Opus 5.5 <[email protected]>
adriangb
pushed a commit
to pydantic/arrow-rs
that referenced
this pull request
Sep 29, 2026
…ing a batch (apache#11218) # Which issue does this PR close? - Related to apache#6946 # Rationale for this change While discussing apache#6946 with @adriangb, a question came up about whether `ParquetPushDecoder` would decode a batch if only the pages needed for that batch had been pushed or if it insists on having all pages for the row group first. This PR adds a test that answers that question For the current code, the ParquetPushDecoder requires that all ranges for the row group are present # What changes are included in this PR? `test_decoder_first_page_only` test # Are these changes tested? This PR is only a test. # Are there any user-facing changes? No. --------- Co-authored-by: Claude Fable 5.1 <[email protected]>
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Oct 1, 2026
test_decoder_first_pages_only_batch_granularity is the counterpart of the test from apache#11218. incremental_tests.rs checks that both granularities give the same batches (targeted and randomized), and checks the requests, the row-group boundary and the bytes held. Co-Authored-By: Claude Opus 5.5 <[email protected]>
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Oct 1, 2026
test_decoder_first_pages_only_batch_granularity is the counterpart of the test from apache#11218. incremental_tests.rs checks that both granularities give the same batches (targeted and randomized), and checks the requests, the row-group boundary and the bytes held. Co-Authored-By: Claude Opus 5.5 <[email protected]>
adriangb
added a commit
to pydantic/arrow-rs
that referenced
this pull request
Oct 9, 2026
test_decoder_first_pages_only_batch_granularity is the counterpart of the test from apache#11218. incremental_tests.rs checks that both granularities give the same batches (targeted and randomized), and checks the requests, the row-group boundary and the bytes held. Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.
Which issue does this PR close?
Rationale for this change
While discussing #6946 with @adriangb, a question came up about whether
ParquetPushDecoderwould decode a batch if only the pages needed for that batch had been pushed or if it insists on having all pages for the row group first.This PR adds a test that answers that question
For the current code, the ParquetPushDecoder requires that all ranges for the row group are present
What changes are included in this PR?
test_decoder_first_page_onlytestAre these changes tested?
This PR is only a test.
Are there any user-facing changes?
No.