Skip to content

Commit 897f9ea

Browse files
adriangbclaude
andcommitted
feat(parquet): observer-driven pruning in row-group and page filters
Both filters expose a fluent, observer-capable prune op alongside the existing methods (kept as thin shims, deprecatable later): filter.prune_row_groups(&ctx, &conjunction) .with_observer(&mut obs) // optional .prune(); let result = filter.prune_pages(access_plan, &ctx) .with_observer(&mut obs) // optional .prune(); The four always-together refs (arrow/parquet schema, metadata/groups, metrics) are bundled into `RowGroupPruningContext` / `PagePruningContext` so no method carries a long argument list. The observer-accepting ops are `pub(crate)` — the consumer (adaptive parquet scan) is in-crate — so the public API stays `prune_by_statistics` / `prune_plan_with_page_index`, unchanged. `RowGroupAccessPlanFilter`: `prune_by_statistics` now wraps the predicate via `PruningConjunction::single` and delegates; fully-matched detection derives the combined expression from the conjunction's `combined_orig_expr()` rather than taking a separate predicate argument. `PagePruningAccessPlanFilter`: predicates carry an optional `Tag` internally (`TaggedPagePredicate`); a builder replaces the old tuple-slice `new_tagged`. The page-index loop emits one observer event per leaf actually evaluated — leaves cut off by the `!selects_any` short-circuit are not observed (resolves reviewer Q2 on apache#22235: stats are not biased by predicates that never ran). The untagged and observer paths share one body (`run_prune`); no duplicated method. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 218c19b commit 897f9ea

3 files changed

Lines changed: 479 additions & 109 deletions

File tree

datafusion/datasource-parquet/src/opener/mod.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use self::early_stop::EarlyStoppingStream;
2525
use self::encryption::EncryptionContext;
2626
use crate::access_plan::PreparedAccessPlan;
2727
use crate::decoder_projection::DecoderProjection;
28-
use crate::page_filter::PagePruningAccessPlanFilter;
28+
use crate::page_filter::{PagePruningAccessPlanFilter, PagePruningContext};
2929
use crate::push_decoder::{DecoderBuilderConfig, PushDecoderStreamState};
3030
use crate::row_filter::RowFilterGenerator;
3131
use crate::row_group_filter::{BloomFilterStatistics, RowGroupAccessPlanFilter};
@@ -1103,14 +1103,15 @@ impl RowGroupsPrunedParquetOpen {
11031103
&& !access_plan.is_empty()
11041104
&& let Some(page_pruning_predicate) = page_pruning_predicate
11051105
{
1106+
let ctx = PagePruningContext {
1107+
arrow_schema: &prepared.physical_file_schema,
1108+
parquet_schema: reader_metadata.parquet_schema(),
1109+
parquet_metadata: file_metadata.as_ref(),
1110+
file_metrics: &prepared.file_metrics,
1111+
};
11061112
let page_pruning_result = page_pruning_predicate
1107-
.prune_plan_with_page_index_and_metrics(
1108-
access_plan,
1109-
&prepared.physical_file_schema,
1110-
reader_metadata.parquet_schema(),
1111-
file_metadata.as_ref(),
1112-
&prepared.file_metrics,
1113-
);
1113+
.prune_pages(access_plan, &ctx)
1114+
.prune();
11141115
access_plan = page_pruning_result.access_plan;
11151116
ParquetFileMetrics::add_page_index_pages_skipped_by_fully_matched(
11161117
&prepared.metrics,

datafusion/datasource-parquet/src/page_filter.rs

Lines changed: 241 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use arrow::{
3131
use datafusion_common::ScalarValue;
3232
use datafusion_common::pruning::PruningStatistics;
3333
use datafusion_physical_expr::{PhysicalExpr, split_conjunction};
34-
use datafusion_pruning::PruningPredicate;
34+
use datafusion_pruning::{NoopObserver, PruningObserver, PruningPredicate, Tag};
3535

3636
use log::{debug, trace};
3737
use parquet::arrow::arrow_reader::statistics::StatisticsConverter;
@@ -110,9 +110,61 @@ use parquet::{
110110
/// row selection that is added to the [`ParquetAccessPlan`].
111111
#[derive(Debug)]
112112
pub struct PagePruningAccessPlanFilter {
113-
/// single column predicates (e.g. (`col = 5`) extracted from the overall
114-
/// predicate. Must all be true for a row to be included in the result.
115-
predicates: Vec<PruningPredicate>,
113+
/// Single-column conjuncts split from the top-level `AND` of the
114+
/// overall predicate (via `split_conjunction`), keeping only those
115+
/// that build into a non-trivial single-column `PruningPredicate`
116+
/// (e.g. `col = 5`, `col < 10`, or a same-column `col < 5 OR col > 100`).
117+
/// Multi-column, always-true, and non-buildable conjuncts are
118+
/// dropped, so this is a *necessary-but-not-sufficient* subset of
119+
/// the filter: every entry must hold for a row to be included, but
120+
/// satisfying all of them does not by itself guarantee inclusion.
121+
/// Page pruning ANDs each entry's per-page selection — a page is
122+
/// kept only if it may satisfy all of them.
123+
///
124+
/// Each carries an optional caller [`Tag`]; when present, the
125+
/// page-index evaluation loop fires `observer.on_leaf(tag, ..)`
126+
/// after evaluating that predicate. Untagged predicates produce
127+
/// `None` tags and no-op against `NoopObserver`.
128+
predicates: Vec<TaggedPagePredicate>,
129+
}
130+
131+
/// Single-column predicate paired with an optional caller tag.
132+
#[derive(Debug)]
133+
struct TaggedPagePredicate {
134+
tag: Option<Tag>,
135+
predicate: PruningPredicate,
136+
}
137+
138+
/// Builder for a tagged [`PagePruningAccessPlanFilter`]. Each
139+
/// [`push`](Self::push) runs the same single-column / non-trivial
140+
/// filtering as [`PagePruningAccessPlanFilter::new`]; conjuncts that
141+
/// fail it are dropped (their tags will not appear in observer
142+
/// output). [`build`](Self::build) always succeeds — an empty filter
143+
/// is a valid no-op pruner.
144+
#[derive(Debug)]
145+
pub struct PagePruningAccessPlanFilterBuilder {
146+
schema: SchemaRef,
147+
predicates: Vec<TaggedPagePredicate>,
148+
}
149+
150+
impl PagePruningAccessPlanFilterBuilder {
151+
/// Add a tagged conjunct. Mutates and returns `&mut self`, so this
152+
/// works in both chains and loops.
153+
pub fn push(&mut self, tag: Tag, expr: Arc<dyn PhysicalExpr>) -> &mut Self {
154+
if let Some(p) =
155+
PagePruningAccessPlanFilter::build_one(expr, &self.schema, Some(tag))
156+
{
157+
self.predicates.push(p);
158+
}
159+
self
160+
}
161+
162+
/// Finish building.
163+
pub fn build(self) -> PagePruningAccessPlanFilter {
164+
PagePruningAccessPlanFilter {
165+
predicates: self.predicates,
166+
}
167+
}
116168
}
117169

118170
/// Result of applying page-index pruning to a [`ParquetAccessPlan`].
@@ -135,44 +187,129 @@ impl PagePruningResult {
135187
}
136188
}
137189

190+
/// The always-together context for a page-index prune. Bundled so the
191+
/// prune entry points don't carry four loose refs.
192+
#[derive(Clone, Copy)]
193+
pub(crate) struct PagePruningContext<'a> {
194+
pub arrow_schema: &'a Schema,
195+
pub parquet_schema: &'a SchemaDescriptor,
196+
pub parquet_metadata: &'a ParquetMetaData,
197+
pub file_metrics: &'a ParquetFileMetrics,
198+
}
199+
200+
/// A configured-but-not-yet-run page-index prune, produced by
201+
/// [`PagePruningAccessPlanFilter::prune_pages`]. Optionally attach an
202+
/// observer with [`with_observer`](Self::with_observer), then execute
203+
/// with [`prune`](Self::prune).
204+
pub(crate) struct PagePrune<'a> {
205+
filter: &'a PagePruningAccessPlanFilter,
206+
access_plan: ParquetAccessPlan,
207+
ctx: &'a PagePruningContext<'a>,
208+
observer: Option<&'a mut dyn PruningObserver>,
209+
}
210+
211+
impl<'a> PagePrune<'a> {
212+
/// Attach a per-leaf observer. Omit to prune without collecting
213+
/// per-conjunct stats (zero overhead).
214+
// Consumed by the adaptive parquet scan (later in the stack); the
215+
// untagged `prune_plan_with_page_index` path never sets an observer.
216+
#[expect(
217+
dead_code,
218+
reason = "tagged page-prune consumer lands later in the stack"
219+
)]
220+
pub fn with_observer(mut self, observer: &'a mut dyn PruningObserver) -> Self {
221+
self.observer = Some(observer);
222+
self
223+
}
224+
225+
/// Run the prune, returning the updated access plan and metrics.
226+
pub fn prune(self) -> PagePruningResult {
227+
let PagePrune {
228+
filter,
229+
access_plan,
230+
ctx,
231+
observer,
232+
} = self;
233+
let mut noop = NoopObserver;
234+
let observer: &mut dyn PruningObserver = match observer {
235+
Some(o) => o,
236+
None => &mut noop,
237+
};
238+
filter.run_prune(access_plan, ctx, observer)
239+
}
240+
}
241+
138242
impl PagePruningAccessPlanFilter {
139243
/// Create a new [`PagePruningAccessPlanFilter`] from a physical
140-
/// expression.
244+
/// expression. Predicates created this way have no caller tag.
141245
#[expect(clippy::needless_pass_by_value)]
142246
pub fn new(expr: &Arc<dyn PhysicalExpr>, schema: SchemaRef) -> Self {
143-
// extract any single column predicates
144247
let predicates = split_conjunction(expr)
145248
.into_iter()
146-
.filter_map(|predicate| {
147-
let pp = match PruningPredicate::try_new(
148-
Arc::clone(predicate),
149-
Arc::clone(&schema),
150-
) {
151-
Ok(pp) => pp,
152-
Err(e) => {
153-
debug!("Ignoring error creating page pruning predicate: {e}");
154-
return None;
155-
}
156-
};
157-
158-
if pp.always_true() {
159-
debug!("Ignoring always true page pruning predicate: {predicate}");
160-
return None;
161-
}
162-
163-
if pp.required_columns().single_column().is_none() {
164-
debug!("Ignoring multi-column page pruning predicate: {predicate}");
165-
return None;
166-
}
167-
168-
Some(pp)
169-
})
249+
.filter_map(|predicate| Self::build_one(Arc::clone(predicate), &schema, None))
170250
.collect::<Vec<_>>();
171251
Self { predicates }
172252
}
173253

254+
/// Start building a filter whose conjuncts each carry a
255+
/// caller-supplied [`Tag`]. Mirrors [`PruningConjunction::builder`]:
256+
///
257+
/// ```ignore
258+
/// let mut b = PagePruningAccessPlanFilter::builder(schema);
259+
/// for (id, expr) in conjuncts {
260+
/// b.push(id, expr);
261+
/// }
262+
/// let filter = b.build();
263+
/// ```
264+
///
265+
/// During pruning the page-index loop fires `observer.on_leaf(tag,
266+
/// ..)` once per leaf actually evaluated; leaves cut off by the
267+
/// AND short-circuit on row selection (`!selects_any`) are not
268+
/// observed.
269+
///
270+
/// [`PruningConjunction::builder`]: datafusion_pruning::PruningConjunction::builder
271+
pub fn builder(schema: SchemaRef) -> PagePruningAccessPlanFilterBuilder {
272+
PagePruningAccessPlanFilterBuilder {
273+
schema,
274+
predicates: Vec::new(),
275+
}
276+
}
277+
278+
fn build_one(
279+
expr: Arc<dyn PhysicalExpr>,
280+
schema: &SchemaRef,
281+
tag: Option<Tag>,
282+
) -> Option<TaggedPagePredicate> {
283+
let pp = match PruningPredicate::try_new(expr, Arc::clone(schema)) {
284+
Ok(pp) => pp,
285+
Err(e) => {
286+
debug!("Ignoring error creating page pruning predicate: {e}");
287+
return None;
288+
}
289+
};
290+
if pp.always_true() {
291+
debug!(
292+
"Ignoring always true page pruning predicate: {}",
293+
pp.orig_expr()
294+
);
295+
return None;
296+
}
297+
if pp.required_columns().single_column().is_none() {
298+
debug!(
299+
"Ignoring multi-column page pruning predicate: {}",
300+
pp.orig_expr()
301+
);
302+
return None;
303+
}
304+
Some(TaggedPagePredicate { tag, predicate: pp })
305+
}
306+
174307
/// Returns an updated [`ParquetAccessPlan`] by applying predicates to the
175-
/// parquet page index, if any
308+
/// parquet page index, if any.
309+
///
310+
/// Thin shim over the fluent `prune_pages` op (no observer).
311+
/// Could be `#[deprecated]` in favor of the fluent form once the
312+
/// adaptive scan migrates.
176313
pub fn prune_plan_with_page_index(
177314
&self,
178315
access_plan: ParquetAccessPlan,
@@ -181,26 +318,55 @@ impl PagePruningAccessPlanFilter {
181318
parquet_metadata: &ParquetMetaData,
182319
file_metrics: &ParquetFileMetrics,
183320
) -> ParquetAccessPlan {
184-
self.prune_plan_with_page_index_and_metrics(
185-
access_plan,
321+
let ctx = PagePruningContext {
186322
arrow_schema,
187323
parquet_schema,
188324
parquet_metadata,
189325
file_metrics,
190-
)
191-
.access_plan
326+
};
327+
self.prune_pages(access_plan, &ctx).prune().access_plan
328+
}
329+
330+
/// Begin a page-index prune. Returns a [`PagePrune`] op that can
331+
/// optionally take an observer before running:
332+
///
333+
/// ```ignore
334+
/// let result = filter.prune_pages(access_plan, &ctx)
335+
/// .with_observer(&mut obs) // optional
336+
/// .prune();
337+
/// ```
338+
pub(crate) fn prune_pages<'a>(
339+
&'a self,
340+
access_plan: ParquetAccessPlan,
341+
ctx: &'a PagePruningContext<'a>,
342+
) -> PagePrune<'a> {
343+
PagePrune {
344+
filter: self,
345+
access_plan,
346+
ctx,
347+
observer: None,
348+
}
192349
}
193350

194-
/// Returns an updated [`ParquetAccessPlan`] and metrics by applying predicates
195-
/// to the parquet page index, if any.
196-
pub(crate) fn prune_plan_with_page_index_and_metrics(
351+
/// Workhorse for page-index pruning. The observer fires
352+
/// `on_leaf(tag, mask)` once per predicate that is actually
353+
/// evaluated; `mask[i] = true` means row group `i` still has at
354+
/// least one page that may match that leaf. Predicates skipped by
355+
/// the per-row-group `!selects_any` short-circuit are not observed,
356+
/// so per-conjunct stats are not biased by predicates that never
357+
/// ran (resolves the reviewer's Q2 concern on PR #22235).
358+
fn run_prune(
197359
&self,
198360
mut access_plan: ParquetAccessPlan,
199-
arrow_schema: &Schema,
200-
parquet_schema: &SchemaDescriptor,
201-
parquet_metadata: &ParquetMetaData,
202-
file_metrics: &ParquetFileMetrics,
361+
ctx: &PagePruningContext<'_>,
362+
observer: &mut dyn PruningObserver,
203363
) -> PagePruningResult {
364+
let PagePruningContext {
365+
arrow_schema,
366+
parquet_schema,
367+
parquet_metadata,
368+
file_metrics,
369+
} = *ctx;
204370
// scoped timer updates on drop
205371
let _timer_guard = file_metrics.page_index_eval_time.timer();
206372
if self.predicates.is_empty() {
@@ -210,6 +376,16 @@ impl PagePruningAccessPlanFilter {
210376
let page_index_predicates = &self.predicates;
211377
let groups = parquet_metadata.row_groups();
212378

379+
// Per-leaf "did this row group still have any matching pages
380+
// after the leaf alone?" mask. Built across all row groups,
381+
// emitted via `observer.on_leaf` at the end so each leaf gets
382+
// exactly one observation per call to this function. Leaves
383+
// never evaluated (because an earlier conjunct emptied the
384+
// running row selection — `!selects_any` break below) end up
385+
// with `None` here and are intentionally not observed.
386+
let mut per_leaf_mask: Vec<Option<Vec<bool>>> =
387+
(0..page_index_predicates.len()).map(|_| None).collect();
388+
213389
if groups.is_empty() {
214390
return PagePruningResult::new(access_plan, 0);
215391
}
@@ -262,7 +438,8 @@ impl PagePruningAccessPlanFilter {
262438
let mut matched_pages_in_group: HashSet<usize> =
263439
HashSet::from_iter(0..total_pages_in_group);
264440

265-
for predicate in page_index_predicates {
441+
for (leaf_idx, tagged) in page_index_predicates.iter().enumerate() {
442+
let predicate = &tagged.predicate;
266443
let Some(column) = predicate.required_columns().single_column() else {
267444
debug!(
268445
"Ignoring multi-column page pruning predicate: {:?}",
@@ -307,6 +484,15 @@ impl PagePruningAccessPlanFilter {
307484
predicate.predicate_expr(),
308485
);
309486

487+
// Per-leaf observation: this leaf ran for `row_group_index`
488+
// and produced `selection`. Whether this row group is
489+
// "kept" by the leaf is `selection.selects_any()`. The
490+
// entry is created lazily so leaves never observed (due
491+
// to short-circuit) stay `None`.
492+
let mask = per_leaf_mask[leaf_idx]
493+
.get_or_insert_with(|| vec![false; groups.len()]);
494+
mask[row_group_index] = selection.selects_any();
495+
310496
let matched_pages_indexes: HashSet<_> = pages
311497
.into_iter()
312498
.enumerate()
@@ -369,6 +555,18 @@ impl PagePruningAccessPlanFilter {
369555
file_metrics
370556
.page_index_pages_pruned
371557
.add_matched(total_pages_select);
558+
559+
// Emit one observer event per leaf that was actually
560+
// evaluated against at least one row group. Leaves that the
561+
// outer `!selects_any` short-circuit prevented from running
562+
// stay `None` here and are correctly absent from the stats.
563+
for (leaf_idx, mask_opt) in per_leaf_mask.into_iter().enumerate() {
564+
if let Some(mask) = mask_opt {
565+
let tag = page_index_predicates[leaf_idx].tag;
566+
observer.on_leaf(tag, &mask);
567+
}
568+
}
569+
372570
PagePruningResult::new(access_plan, total_pages_skipped_by_fully_matched)
373571
}
374572

0 commit comments

Comments
 (0)