perf: factor common guards out of pruning predicates - #24796
Draft
mattp5657 wants to merge 1 commit into
Draft
Conversation
mattp5657
force-pushed
the
perf/pruning-factor-common-guards
branch
from
August 30, 2026 10:52
1dfa2c1 to
c8e5e33
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24796 +/- ##
==========================================
+ Coverage 81.53% 81.55% +0.02%
==========================================
Files 1123 1123
Lines 406041 406818 +777
Branches 406041 406818 +777
==========================================
+ Hits 331049 331793 +744
- Misses 55631 55660 +29
- Partials 19361 19365 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mattp5657
marked this pull request as draft
August 30, 2026 13:21
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #24280.
Rationale for this change
wrap_null_count_check_exprattaches a null-count guard(
col_null_count != row_count) to every rewritten leaf comparisonindependently, so a predicate that lowers to more than one leaf on the
same column (e.g. a
BETWEEN-shaped range) duplicates its guard:Both copies get evaluated against every row group's stats at prune time.
Under three-valued logic,
(G AND P) AND (G AND Q) == G AND P AND Qand(G AND P) OR (G AND Q) == G AND (P OR Q), so the guard can be hoistedand evaluated once instead of once per leaf: same pruning decision,
less redundant work to reach it.
What changes are included in this PR?
A
factor_common_guardspass, run once inPruningPredicateBuilder:: try_build, applying that identity toAND/ORnodes:flatten_chain_known_non_volatileiteratively flattens nestedsame-operator chains (no recursion, no stack-overflow risk on a long
chain).
factor_anddedupes structurally-identical arms viaHashSet.factor_orhoists whatever conjunct is common to every arm of anOR. Only hoists over the full arm set, not subsets: e.g.(g AND a=1) OR (g AND a=2) OR (g AND a=3) OR b IS NULLgets no benefit eventhough 3 of 4 arms share
g. Never incorrect, just a smaller win.fold_and/fold_orrebuild the tree, short-circuiting on analways-true/false arm.
Skipped entirely if any part of the expression is volatile
(
is_volatile), since the identity only holds for deterministicpredicates. Recursion is capped at
MAX_FACTOR_ALTERNATION_DEPTH = 32operator alternations:
Arc<dyn PhysicalExpr>'sEq/Hashare fullystructural with no pointer-identity shortcut, so hashing a subtree of
size
ScostsO(S), and recursing per alternation on a chain of depthNisO(N^2)worst case (observed worse in practice on a syntheticdeep tree). Past the cap, the rest is left unfactored (always correct,
same fallback already used for opaque non-
BinaryExprleaves).Are these changes tested?
Yes.
cargo test -p datafusion-pruning --lib pruning_predicatepasses115 tests (1 unrelated filtered out), including new coverage for: the
volatile-expression bailout, an 8,000-arm flatten,
OR-hoisting withmultiple/compound common conjuncts, a 500-level alternating
AND/ORtree (depth cap, no stack overflow), and a new
i IN (11, NULL)equivalence case.
9
.sltgolden files needed re-recording since the factored predicate'sEXPLAINtext differs (guard printed once, not twice) with no change inreturned rows:
clickbench,explain_analyze,limit_pruning,parquet_filter_pushdown,projection_pushdown,push_down_filter_parquet,push_down_filter_regression,range_partitioning,sort_pushdown.Are there any user-facing changes?
No behavioral changes.
EXPLAIN/EXPLAIN ANALYZEtext changes (guardprinted once instead of twice), which is why the
.sltfiles aboveneeded updating. No change to query results, pruning decisions, or plan
shape.
Measurements
TPC-H: no measurable effect on query time
TPC-H SF=1, parquet, 8 cores,
target_partitions=8, two binaries fromthe same worktree (
cmp-verified to differ), 12 counterbalanced roundsx 5 iterations, at two row-group scales on
lineitem(53 and 3,840, a72x spread). Positive % means the change is slower; "order-dominated"
means the two counterbalanced orderings disagreed enough that no
conclusion is drawn either way, reported honestly as unresolved rather
than averaged into a false-precision number.
No measurable effect on wall-clock query time, in either direction, at
either scale. Control floor (q1/q13/q18, byte-identical plans on both
binaries) is ~0.8% in both runs; nothing clears 1.5x that floor
sign-consistently across both counterbalanced orderings, except one
marginal hit (default-scale q4, +1.38%) that fails to reproduce at the
other scale (-0.20%), exactly what noise looks like across 22 queries
tested per run, not a regression.
Run 1: default row-group size (53 row groups), control floor 0.81%
Run 2: 3,840 row groups on
lineitem(72x the default), independent re-run, control floor 0.82%Mechanical evidence (
dfbench --debug, q6'slineitemscan): theduplicated
l_shipdate/l_discountnull-count guards each collapsefrom 2 occurrences to 1, at both row-group scales, with row-groups-matched
and bytes-scanned byte-identical before and after. The redundant
comparisons this PR removes are real and confirmed structurally.
Microbenchmark: a real, tiny improvement
Trying to measure the eval-time saving directly from TPC-H's own
statistics_eval_timemetric didn't work: it's contaminated by anunrelated timing sensitivity in hash-join dynamic-filter resolution that
swamps a signal this small (full investigation in
.ai/plans/benchmark-writeup-final.md). The right tool for "how muchdoes this one function save" is a microbenchmark, not a whole-query
measurement:
benchmarks/benches/pruning_guard_dedup.rs(criterion)builds the same predicate with and without
factor_common_guards, inone process, and calls
PruningPredicate::pruneagainst syntheticcontainer stats directly: no parquet I/O, no joins, no dynamic filters,
nothing else that could confound it.
Result, on a q6-shaped predicate (2 guards duplicated, 1 not: 5 guard
occurrences before factoring, 3 after).
PruningStatistics:: num_containersis generic (files, partitions, etc. elsewhere inDataFusion), but here each container is a row group, matching the same
row-group counts used in the TPC-H runs above:
Non-overlapping confidence intervals at every size. This is a real,
reproducible saving, and it scales up with container count exactly as
the mechanism predicts. It's also genuinely tiny: microseconds, against
individual query times of tens to hundreds of milliseconds. That's why
the TPC-H wall-clock benchmark above can't see it in either direction:
it's not that there's no effect, it's that the effect is real and far
too small to matter at query scale.