Skip to content

feat(parquet): multi-column lexicographic stats reorder for TopK sort pushdown#23888

Draft
zhuqi-lucas wants to merge 1 commit into
apache:mainfrom
zhuqi-lucas:feat/multicol-topk-reorder
Draft

feat(parquet): multi-column lexicographic stats reorder for TopK sort pushdown#23888
zhuqi-lucas wants to merge 1 commit into
apache:mainfrom
zhuqi-lucas:feat/multicol-topk-reorder

Conversation

@zhuqi-lucas

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

PreparedAccessPlan::reorder_by_statistics (row-group level) and ParquetSource::reorder_files (file level) key only off the leading sort column's min statistic. When the leading key ties across row groups or files — the ORDER BY low_cardinality_col, ts LIMIT k shape — the reorder degenerates to a no-op, the scan reads data in disk order, and the TopK dynamic filter's threshold improves monotonically without ever proving a later row group unwinnable. Result: zero runtime RG pruning and a full-file decode even though per-column statistics contain everything needed to read the best row group first.

The end-to-end test in this PR demonstrates the failure concretely on current main: leading key tied across 5 row groups, secondary key clustered but stored in adversarial DESC disk order → row_groups_pruned_dynamic_filter=0, all 500 rows decoded. With this change the scan reads the best RG first and prunes the other 4.

Note the pruning machinery itself already handles multi-column lexicographic dynamic filters (the leading disjunct of a < x OR (a = x AND b < y) prunes on min(a) stats — verified by the other new e2e test, which passes on main unmodified). The gap was purely in the reorder layers feeding it.

What changes are included in this PR?

Both reorder layers now sort lexicographically over the longest plain-Column prefix of the sort order, using per-column min statistics:

  • RG level (access_plan.rs): switch from sort_to_indices over the leading column's mins to lexsort_to_indices over per-column min arrays. The leading key stays normalized ASC (direction still applied by the downstream reverse(), unchanged); secondary keys sort by their direction relative to the leading key, with null placement pre-flipped when the reverse is coming so the post-reverse order matches the request. The prefix walk stops at the first non-Column / not-in-file-schema expression (leading-key-only graceful skips unchanged).
  • File level (sort.rs): extract_topk_sort_info extended from the single leading column to the plain-Column prefix, comparator compares tuple-wise with per-column direction; missing-stats files still sort last per column.

Behavior for single-column sort orders is unchanged.

Are these changes tested?

  • 4 new unit tests for reorder_by_statistics: secondary tie-break, relative secondary direction (a ASC, b DESC), DESC/DESC normalization through reverse(), and prefix-stop on a non-Column secondary expression.
  • 1 new unit test for file-level reorder: secondary tie-break with missing-stats file placement.
  • 2 new end-to-end tests in dynamic_row_group_pruning.rs:
    • multi-column sort with clustered leading key → pruning fires via the leading disjunct (passes before this PR; guards the existing behavior),
    • multi-column sort with tied leading key + adversarial disk order → pruning fires only with this PR's lexicographic reorder.
  • Existing reorder unit tests, dynamic_row_group_pruning integration tests, and topk.slt / sort_pushdown.slt / dynamic_row_group_pruning.slt / dynamic_filter_pushdown_config.slt all pass.
  • topk_tpch (sort-tpch -l 100) shows no regressions (all 11 queries within noise; lineitem's secondary keys are not clustered when the leading key ties, so no wins expected on that suite — the win case is the ORDER BY low_card, ts shape shown in the e2e test).

Are there any user-facing changes?

No API changes. Queries with multi-column ORDER BY ... LIMIT over parquet whose leading sort key ties across row groups / files can now skip row groups at runtime instead of decoding the whole file.

… pushdown

Implements apache#22198 P0: PreparedAccessPlan::reorder_by_statistics and
ParquetSource::reorder_files previously keyed only off the leading sort
column's min statistic. When the leading key ties across row groups or
files (e.g. ORDER BY low_cardinality_col, ts LIMIT k), the reorder was a
no-op, the scan read row groups in disk order, and the TopK dynamic
filter never tightened enough for the runtime RG pruner to fire.

Both layers now sort lexicographically over the longest plain-Column
prefix of the sort order, by per-column min with per-column directions
expressed relative to the leading key (the leading key stays normalized
ASC at the RG level; direction is applied by the later reverse() as
before).

The included end-to-end test shows the effect: leading key tied across
5 row groups, secondary key clustered but stored in adversarial DESC
disk order — before this change row_groups_pruned_dynamic_filter=0 and
all 500 rows decode; after it, the scan reads the best RG first and
prunes the other 4.
Copilot AI review requested due to automatic review settings July 25, 2026 15:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added core Core DataFusion crate datasource Changes to the datasource crate labels Jul 25, 2026
@zhuqi-lucas
zhuqi-lucas marked this pull request as draft July 25, 2026 15:48
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.90283% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.66%. Comparing base (8393fd3) to head (1187306).

Files with missing lines Patch % Lines
datafusion/datasource-parquet/src/access_plan.rs 89.41% 18 Missing ⚠️
datafusion/datasource-parquet/src/sort.rs 93.93% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #23888    +/-   ##
========================================
  Coverage   80.65%   80.66%            
========================================
  Files        1092     1092            
  Lines      371106   371300   +194     
  Branches   371106   371300   +194     
========================================
+ Hits       299323   299508   +185     
- Misses      53937    53943     +6     
- Partials    17846    17849     +3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate datasource Changes to the datasource crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants