haohuaijin opened a new pull request, #10434:
URL: https://github.com/apache/arrow-rs/pull/10434
# Which issue does this PR close?
- Closes #10424.
# Rationale for this change
Follow up to #10141. `parquet/src/arrow/arrow_reader/selection/mod.rs` had
grown past 2100 lines and mixed several concerns: the `RowSelection` type and
its public API, the set algebra helpers, the page range mapping, the cursor
machinery and a large test module.
This is a mechanical reorganization to make the code easier to navigate and
review.
# What changes are included in this PR?
The module is split by concern, with one module per `RowSelection` backing
so the two are symmetric:
| module | contents |
| --- | --- |
| `mod.rs` | `RowSelection`, `RowSelectionInner` and the public API,
dispatching on the backing |
| `selector.rs` | run length backing: `RowSelector`, `RowSelectionIter` and
the selector primitives |
| `boolean.rs` | bitmap backing: `MaskSelection`, `MaskRunIter` and the mask
primitives |
| `algebra.rs` | `and_then`, `intersection`, `union` for both backings |
| `ranges.rs` | page byte ranges and batch boundary expansion |
| `cursor.rs` | `RowSelectionCursor`, `MaskCursor`, `SelectorsCursor`,
`LoadedRowRanges`, `RowSelectionPolicy` / `RowSelectionStrategy` |
The two backings now mirror each other:
| operation | `selector.rs` | `boolean.rs` |
| --- | --- | --- |
| storage | `Vec<RowSelector>` | `MaskSelection` |
| iteration | `RowSelectionIter` | `MaskRunIter` |
| construction | `combine_selectors`, `selectors_from_consecutive_ranges` |
`boolean_mask_from_selectors` |
| split / trim / offset / limit | `*_selectors` | `*_mask` |
so each `RowSelection` method is a short dispatch on `RowSelectionInner`,
for example:
```rust
pub(crate) fn limit(self, limit: usize) -> Self {
match self.inner {
RowSelectionInner::Mask(mask) => { /* limit_mask + count bookkeeping
*/ }
RowSelectionInner::Selectors(selectors) => {
Self::from_selectors(limit_selectors(selectors, limit))
}
}
}
```
Function bodies are moved verbatim. The only shape changes are:
* the selector helpers return `Vec<RowSelector>` instead of `RowSelection`,
so the wrapping happens once at the call site;
* `RowSelectionIter` gained a `pub(super) fn new`, as its tuple field is no
longer visible from `mod.rs`;
* `and_then_selectors_with_mask` was added to `algebra.rs` (a 6 line
wrapper) so that `and_then_iter` can stay private to that module.
# Are these changes tested?
Yes, by the existing tests: they were moved next to the code they now cover
(selector mechanics to `selector.rs`, algebra to `algebra.rs`, `scan_ranges` to
`ranges.rs`, cursor tests to `cursor.rs`), but no test was added, removed or
renamed — the set of test names under `arrow::arrow_reader::selection` is
identical before and after this PR.
# Are there any user-facing changes?
No. The public API (`parquet::arrow::arrow_reader::{RowSelection,
RowSelector, RowSelectionCursor, RowSelectionPolicy, RowSelectionIter,
MaskRunIter}`) is unchanged, and no call site outside `selection/` needed
modification.
--
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]
