Copilot commented on code in PR #11165:
URL: https://github.com/apache/arrow-rs/pull/11165#discussion_r4074220989
##########
parquet/src/arrow/push_decoder/mod.rs:
##########
@@ -427,6 +427,20 @@ pub struct ParquetPushDecoder {
state: ParquetDecoderState,
}
+/// A metadata-only snapshot of one row group's next required byte ranges.
+///
+/// No payload or decoded arrays are retained. A caller must compare these
ranges
+/// with ordered `NeedsData` demand before consuming speculative I/O: pushing,
+/// consuming, or clearing buffered data, or rebuilding the decoder, can
+/// invalidate a snapshot.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct RowGroupRangePreview {
+ /// File-level row-group index, after selection and offset/limit skipping.
+ pub row_group_index: usize,
+ /// Planned ranges not fully covered by an input buffer.
+ pub ranges: Vec<Range<u64>>,
+}
Review Comment:
Because `RowGroupRangePreview` is a public struct with public fields, adding
fields later will be a breaking change. Consider marking the struct
`#[non_exhaustive]` and/or making fields private with accessor methods to
preserve forward compatibility.
##########
parquet/src/arrow/push_decoder/remaining.rs:
##########
@@ -446,6 +447,36 @@ impl RemainingRowGroups {
self.frontier.peek_next_row_group()
}
+ /// Preview only at a filter-free boundary; the cloned frontier preserves
+ /// selection and offset/limit accounting without advancing ordered demand.
+ pub fn preview_row_group_ranges(
Review Comment:
`RemainingRowGroups::preview_row_group_ranges` is `pub` but appears intended
as an internal helper behind `ParquetPushDecoder::preview_row_group_ranges`
(which enforces `max_row_groups` = 1 or 2). To avoid accidental external use
with unbounded `max_row_groups` (and potentially large allocations), make this
`pub(crate)` (or private), or enforce the same argument bounds here as well.
##########
parquet/src/arrow/push_decoder/mod.rs:
##########
@@ -605,6 +619,36 @@ impl ParquetPushDecoder {
self.state.peek_next_row_group()
}
+ /// Preview at most two filter-free row groups using the demand range
planner.
+ ///
+ /// Returns `None` outside a row-group boundary or when row predicates are
+ /// present; returns an empty vector when there is no selected work. This
+ /// never advances the decoder, evaluates predicates, reads data, or
creates
+ /// batch readers. Ranges fully covered by an input buffer are excluded;
+ /// partially covered ranges are returned unchanged, matching `NeedsData`.
+ ///
+ /// `max_row_groups` must be 1 or 2. Errors leave the decoder unchanged;
+ /// speculative callers should defer them to normal ordered demand. The
+ /// snapshot is only advisory: see [`RowGroupRangePreview`]. Cost includes
+ /// cloning the remaining row-group plan and selections.
+ pub fn preview_row_group_ranges(
+ &self,
+ max_row_groups: usize,
+ ) -> Result<Option<Vec<RowGroupRangePreview>>, ParquetError> {
+ if !(1..=2).contains(&max_row_groups) {
+ return Err(ParquetError::General(
+ "range preview depth must be 1 or 2".into(),
Review Comment:
The error message would be more actionable if it included the actual invalid
value (e.g., \"range preview depth must be 1 or 2, got {max_row_groups}\") so
callers can diagnose misconfiguration more quickly.
--
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]
