zhuqi-lucas commented on PR #10158: URL: https://github.com/apache/arrow-rs/pull/10158#issuecomment-4786279276
Heard you @alamb — the PR did drift into "let's also add a more general API and a visitor framework" territory during the back-and-forth. Pushed `4cdc6d6eb` to trim back to the actual primitive the dynamic-pruner follow-up needs: **Kept** * Single-value `peek_next_row_group()` * `PeekSelectionCursor` (no-clone walker over `RowSelection`) * `DecodingRowGroup` arm returns the next row group rather than `None` * Doc rewordings applied earlier via GitHub suggestions * 8 peek tests covering basic flow, `with_row_groups`, selection-skip, finished state, OFFSET single-RG skip, OFFSET drains all, mid-row-group `try_decode`, and the read-only invariant **Dropped** * `peek_remaining_row_groups` at every layer (no consumer for it) * `walk_peekable` + `PeekStep` visitor framework (only existed to share a loop body with the multi-value variant) * `RowGroupAction` + `classify_row_group` shared classifier — restored the original `QueuedRowGroupDecision` + `plan_selected_row_group` on `next_readable_row_group`. The Read/Skip rule in `peek_next_row_group` is now a 5-line inline mirror of `plan_selected_row_group` with a comment asking future edits to keep them in lock-step. The existing `test_peek_next_row_group_basic` drives peek and `try_next_reader` interleaved across both row groups, so any drift surfaces immediately. * Eight multi-value tests that only mattered for the now-removed API. Net change vs the previous head: `−280 lines`. 1224 / 1224 parquet tests pass, clippy clean. Happy to cut further if anything still feels like scope creep. -- 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]
