Skip to content

Apply min/max segment pruning to filtered selection ORDER BY col LIMIT n#18692

Open
gortiz wants to merge 1 commit into
apache:masterfrom
gortiz:worktree-minmax-orderby-filter-prune
Open

Apply min/max segment pruning to filtered selection ORDER BY col LIMIT n#18692
gortiz wants to merge 1 commit into
apache:masterfrom
gortiz:worktree-minmax-orderby-filter-prune

Conversation

@gortiz
Copy link
Copy Markdown
Contributor

@gortiz gortiz commented Jun 5, 2026

Problem

For SELECT ... ORDER BY <col> [DESC] LIMIT n on a table range-partitioned on <col>, SelectionQuerySegmentPruner prunes down to the handful of segments the LIMIT needs (an unfiltered "latest n" query touches a couple of segments). Adding any filter — even a trivially-true one on an unrelated column — defeated that pruning, and the query engaged every matching segment.

The cause: SelectionQuerySegmentPruner.isApplicableTo only allowed the order-by/LIMIT pruning when there was no filter, because the pruner sizes its keep-set by accumulating the unfiltered getTotalDocs() until LIMIT + OFFSET — which, with a filter, is an upper bound on matching rows and would prune segments that still hold top-n matches (wrong results).

See #18685 for the full reproduction and analysis.

Change

Extend SelectionQuerySegmentPruner to run with a filter by accumulating a lower bound on each segment's matching rows instead of getTotalDocs():

guaranteedMatchingDocs(S) = getTotalDocs(S)  if S provably FULLY satisfies the filter
                            0                 otherwise
  • "Provably full" is the dual of ColumnValueSegmentPruner's no-match test, computed from the predicate column's min/max for >, >=, <, <=, =, <> (conjunction = all children full; OR = any child full; anything unprovable = false).
  • Because the accumulation only ever under-counts, the boundary is reached only once ≥ LIMIT matching rows provably exist, so a segment is never pruned when it might still hold a top-n row. Straddling/partially-matching segments count 0 but are still kept (they overlap the boundary). An AND (...) with a non-provable conjunct simply degrades to "no pruning" — never wrong.
  • The optimization is skipped when null handling is active for a column (nulls are stored as a default value that pollutes the min/max metadata and the doc count), and a NaN min/max is never treated as a full match.

The execution-time MinMaxValueBasedSelectionOrderByCombineOperator skip already worked with a filter; this restores the plan-time pruning that avoids building/opening the pruned segments at all.

Tests

SelectionQuerySegmentPrunerTest adds coverage for: each comparison operator, ASC/DESC, the straddling-segment counterexample (the case the lower bound exists to protect), the AND-with-non-provable-conjunct degradation, the FLOAT/DOUBLE NaN guard, and the null-handling gate (incl. column-based null handling: non-nullable column still optimizes, nullable does not).

Benchmark

Adds BenchmarkSelectionOrderByFilterPruning (pinot-perf). It toggles enableNullHandling to compare the optimized path against the pre-fix path within one build (null handling gates the optimization off). On a 100-segment table (20k rows/segment), LIMIT 10:

query score
filtered, optimized 0.661 ± 0.051 ms/op
filtered, null handling on (pre-fix) 0.824 ± 0.117 ms/op
no filter (reference) 0.661 ± 0.076 ms/op

With the fix the filtered query reaches parity with the unfiltered one; without it the same query is measurably slower. The gap widens with segment count and per-segment cost.

Closes #18685

🤖 Generated with Claude Code

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 5, 2026

Codecov Report

❌ Patch coverage is 85.05747% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.51%. Comparing base (9f6cff8) to head (e0ae4d2).
⚠️ Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
...core/query/pruner/SelectionQuerySegmentPruner.java 85.05% 4 Missing and 9 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18692      +/-   ##
============================================
+ Coverage     64.46%   64.51%   +0.04%     
  Complexity     1291     1291              
============================================
  Files          3371     3372       +1     
  Lines        208551   208721     +170     
  Branches      32569    32621      +52     
============================================
+ Hits         134450   134653     +203     
+ Misses        63292    63266      -26     
+ Partials      10809    10802       -7     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-21 64.51% <85.05%> (+0.04%) ⬆️
temurin 64.51% <85.05%> (+0.04%) ⬆️
unittests 64.51% <85.05%> (+0.04%) ⬆️
unittests1 56.93% <85.05%> (+0.02%) ⬆️
unittests2 37.12% <1.14%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gortiz gortiz force-pushed the worktree-minmax-orderby-filter-prune branch from 73b9172 to e4b78b4 Compare June 5, 2026 12:57
SelectionQuerySegmentPruner only applied its order-by min/max + LIMIT segment
pruning to unfiltered selection queries, so adding any filter to a
`SELECT ... ORDER BY <col> LIMIT n` query defeated the pruning and engaged
every matching segment (apache#18685).

Extend the pruner to run with a filter by counting each segment toward the
LIMIT only the rows that provably match the filter: its total docs when the
segment fully satisfies the filter (the dual of ColumnValueSegmentPruner's
no-match test, computed from min/max metadata for >, >=, <, <=, =, <>), else 0.
Using this lower bound on matching rows keeps the order-by boundary safe, so a
segment is never pruned when it might still hold a top-n matching row; an AND
with a non-provable conjunct simply degrades to no pruning. The optimization is
skipped when null handling is active for a column, because nulls are stored as
a default value that pollutes the min/max metadata, and a NaN min/max is never
treated as a full match.

Add unit tests (range/eq/neq predicates, ASC/DESC, straddling-segment and AND
counterexamples, NaN guard, null-handling gate) and a JMH benchmark
(BenchmarkSelectionOrderByFilterPruning) that toggles null handling to compare
the optimized and pre-fix paths within a single build.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gortiz gortiz force-pushed the worktree-minmax-orderby-filter-prune branch from e4b78b4 to e0ae4d2 Compare June 6, 2026 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Selection ORDER BY <col> LIMIT n does a full segment scan when any filter is present (min/max segment-skip disabled)

2 participants