alanprot opened a new issue, #10648:
URL: https://github.com/apache/arrow-rs/issues/10648
### Describe the bug
### Describe the bug
`ParquetRecordBatchReader::try_new_with_row_groups` regressed in **57.1.0**
and is
still broken on `main`: it can no longer be used with a `RowGroups` that
holds **only**
the data pages its `RowSelection` touches. Before 57.1.0 the reader had a
single
selection strategy, which skips absent pages correctly via `skip_records()`.
#8733 added the Mask strategy and made `Auto` the default — together with a
guard
that keeps Mask away from page-pruned data — but the guard was only wired
into the
push decoder. This constructor silently began resolving to a strategy its
callers'
data cannot satisfy, and has no way to opt out.
Mechanically, it builds its plan as
```rust
let read_plan =
ReadPlanBuilder::new(batch_size).with_selection(selection).build();
```
so it inherits `RowSelectionPolicy::default()` (`Auto`) and never sets
loaded row
ranges. For a fragmented selection `Auto` resolves to Mask, and
`MaskCursor::next_chunk` takes the unguarded path
(`if self.loaded_row_ranges.is_none()`), where `next_mask_chunk_non_empty`
advances until `selected_rows == batch_size` with no page awareness. The
chunk
therefore crosses whole skipped pages, and `read_mask_batch` decodes them.
The built-in readers are fine because `RowGroupReaderBuilder` calls
`prepare_selection_for_page_skipping`, but that isn't reachable from this
constructor: `RowSelectionPolicy` can't be passed, `LoadedRowRanges` and
`row_ranges_for_selected_pages` are `pub(crate)`, and
`ParquetRecordBatchReader::new` is `pub(crate)`.
> **Note:** this seems to be a similar failure that #9301 fixed for the
adaptive predicate pushdown
> path in 58.0.0 — identical error, different entry point. The guard added
there lives
> in the push decoder, which this constructor does not go through.
### To Reproduce
Create a parquet with one row group, 1000 rows, 5 data pages of 200 rows,
page index present,
`batch_size = 200`. The selection is many one-row runs inside pages 0 and 3,
with
pages 1, 2 and 4 skipped whole — so `Auto` resolves to Mask (`1000 < 402 *
32`; note
skips count toward both sides) while only 2 of the 5 pages are fetched, via
`RowSelection::scan_ranges`. Reading that through `try_new_with_row_groups`
gives:
```
Parquet error: Invalid offset in sparse column chunk data: 8243, no matching
page found.
```
Reading the same selection through `ParquetRecordBatchReaderBuilder`
succeeds. Versions ≤ 57.0.x are unaffected (no Mask strategy).
### Expected behavior
No error. `try_new_with_row_groups` should read a page-pruned `RowGroups`
the way it
did before 57.1.0 and return exactly the selected rows — the same rows
`ParquetRecordBatchReaderBuilder` returns for the same `RowSelection`.
### Additional context
### Possible fix
- **Add an `offset_index` method on the `RowGroups` trait**, so it can be
combined
with the `RowSelection` to work out which row ranges have their pages in
memory, and
confine mask decoding to those ranges.
- **Accept a `RowSelectionPolicy` argument**, so the caller can pass
`Selectors`.
The smallest possible change and it removes the error, but it forfeits mask
execution for every page-pruning caller, and it only helps callers who
know they
have to ask for it — the default stays wrong.
- **Accept `LoadedRowRanges`**, i.e. export the type and make
`with_loaded_row_ranges` public. Keeps mask execution, but the caller then
has to
build the ranges, which means exposing the derivation as well
(`RowSelection::loaded_row_ranges`) — more public surface than the trait
method for
the same result.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
