Skip to content

Establish the high level API for sort pushdown and the optimizer rule and support reverse files and row groups - #19064

Merged
zhuqi-lucas merged 33 commits into
apache:mainfrom
zhuqi-lucas:issue_19059
Dec 17, 2025
Merged

Establish the high level API for sort pushdown and the optimizer rule and support reverse files and row groups #19064
zhuqi-lucas merged 33 commits into
apache:mainfrom
zhuqi-lucas:issue_19059

Conversation

@zhuqi-lucas

@zhuqi-lucaszhuqi-lucas commented Dec 3, 2025

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Establish the high level API for sort pushdown and the optimizer rule. Only re-arrange files and row groups and return Inexact, now support reverse order case, and we don't need to cache anything for this implementation, so it's no memory overhead.

It will have huge performance improvement with dynamic topk pushdown to skip row groups.

Details:

Performance results on ClickBench sorted data: 13ms vs 300ms baseline (23x faster), close to aggressive caching approach (9.8ms) but with much better memory stability details

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

…. Only re-arrange files and row groups and return Inexact.
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) common Related to common crate execution Related to the execution crate proto Related to proto crate datasource Changes to the datasource crate physical-plan Changes to the physical-plan crate labels Dec 3, 2025
@zhuqi-lucas
zhuqi-lucas marked this pull request as ready for review December 3, 2025 09:53
CopilotAI review requested due to automatic review settings December 3, 2025 09:53
@zhuqi-lucaszhuqi-lucas changed the title Draft Establish the high level API for sort pushdown and the optimizer rule. Only re-arrange files and row groups and return Inexact.Establish the high level API for sort pushdown and the optimizer rule and support reverse files and row groups Dec 3, 2025

CopilotAI 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.

Pull request overview

This PR establishes the high-level API for sort pushdown optimization in DataFusion, specifically targeting Parquet files. The optimization improves TopK query performance by reordering files and row groups based on statistics, though it returns inexact ordering (keeping the Sort operator for correctness while enabling early termination).

Key Changes:

  • Added enable_sort_pushdown configuration option (default: true) to control the optimization
  • Implemented the optimizer rule PushdownSort that detects SortExec nodes and pushes sort requirements down to data sources
  • Extended ExecutionPlan, DataSource, and FileSource traits with try_pushdown_sort() methods to support the optimization
  • Added reverse_scan_inexact flag to ParquetSource to reverse row group read order when beneficial

Reviewed changes

Copilot reviewed 25 out of 30 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
docs/source/user-guide/configs.mdAdded documentation for the new enable_sort_pushdown configuration option
datafusion/sqllogictest/test_files/topk.sltUpdated test expectation to show reverse_scan_inexact=true in explain output
datafusion/sqllogictest/test_files/spark/bitwise/bit_count.sltFixed line number formatting in test file
datafusion/sqllogictest/test_files/slt_features.sltFixed line number formatting in test file
datafusion/sqllogictest/test_files/information_schema.sltAdded enable_sort_pushdown to configuration schema output
datafusion/sqllogictest/test_files/explain.sltUpdated test expectations to show PushdownSort optimizer phase
datafusion/sqllogictest/test_files/dynamic_filter_pushdown_config.sltAdded comprehensive integration tests for sort pushdown with various scenarios
datafusion/sqllogictest/test_files/create_external_table.sltUpdated test expectation to show reverse_scan_inexact=true
datafusion/proto/src/logical_plan/file_formats.rsAdded enable_sort_pushdown field to protobuf serialization (with hardcoded value bug)
datafusion/proto/src/generated/datafusion_proto_common.rsAdded enable_sort_pushdown field to generated protobuf structures
datafusion/proto-common/src/to_proto/mod.rsImplemented protobuf serialization for enable_sort_pushdown (with hardcoded value bug)
datafusion/proto-common/src/generated/prost.rsAdded enable_sort_pushdown field to generated protobuf structures
datafusion/proto-common/src/generated/pbjson.rsAdded JSON serialization support for enable_sort_pushdown
datafusion/proto-common/src/from_proto/mod.rsImplemented protobuf deserialization for enable_sort_pushdown (with hardcoded value bug)
datafusion/proto-common/proto/datafusion_common.protoAdded enable_sort_pushdown field to protobuf schema
datafusion/physical-plan/src/execution_plan.rsAdded try_pushdown_sort() trait method to ExecutionPlan
datafusion/physical-optimizer/src/pushdown_sort.rsImplemented the core PushdownSort optimizer rule
datafusion/physical-optimizer/src/optimizer.rsRegistered PushdownSort optimizer in the optimization pipeline
datafusion/physical-optimizer/src/lib.rsExported the new pushdown_sort module
datafusion/execution/src/config.rsAdded session config method (with naming and documentation issues)
datafusion/datasource/src/source.rsAdded try_pushdown_sort() trait method to DataSource and DataSourceExec
datafusion/datasource/src/file_scan_config.rsImplemented sort pushdown logic for FileScanConfig including file reversal
datafusion/datasource/src/file.rsAdded SortOrderPushdownResult enum and try_pushdown_sort() trait method to FileSource
datafusion/datasource-parquet/src/source.rsImplemented sort pushdown for ParquetSource with reverse_scan_inexact flag
datafusion/datasource-parquet/src/opener.rsImplemented row group reversal logic in ParquetOpener
datafusion/core/tests/physical_optimizer/test_utils.rsAdded OptimizationTest harness for testing physical optimizers
datafusion/core/tests/physical_optimizer/pushdown_sort.rsAdded comprehensive unit tests for the PushdownSort optimizer
datafusion/core/tests/physical_optimizer/mod.rsRegistered the pushdown_sort test module
datafusion/common/src/file_options/parquet_writer.rsAdded enable_sort_pushdown field to ParquetOptions destructuring
datafusion/common/src/config.rsAdded enable_sort_pushdown configuration field (with indentation issue)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threaddatafusion/proto/src/logical_plan/file_formats.rs Outdated
Comment threaddatafusion/proto-common/src/to_proto/mod.rs Outdated
Comment threaddatafusion/proto-common/src/from_proto/mod.rs Outdated
Comment threaddatafusion/execution/src/config.rs Outdated
Comment threaddatafusion/execution/src/config.rs Outdated
Comment threaddatafusion/common/src/config.rs Outdated
Comment threaddatafusion/proto/src/logical_plan/file_formats.rs Outdated
Comment threaddatafusion/common/src/config.rs Outdated
Comment threaddatafusion/physical-optimizer/src/pushdown_sort.rs Outdated
Comment threaddatafusion/datasource-parquet/src/opener.rs Outdated
Comment threaddatafusion/sqllogictest/test_files/information_schema.slt Outdated
Comment threaddatafusion/datasource/src/file.rs Outdated
Comment threaddatafusion/datasource-parquet/src/source.rs Outdated
zhuqi-lucasand others added 6 commits December 4, 2025 11:26
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@zhuqi-lucas

Copy link
Copy Markdown
ContributorAuthor

I addressed all comments now, cc @alamb@adriangb.

And the latest run benchmark sql_planner shows no regression.

order: &[PhysicalSortExpr],
) -> Result<SortOrderPushdownResult<Arc<dyn ExecutionPlan>>> {
// RepartitionExec only maintains input order if preserve_order is set
// or if there's only one partition

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.

Is the single partition case handled?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes - maintains_input_order_helper returns true for single partitions, so the check passes and sort pushdown works correctly.

Comment on lines +214 to +216
// Default: clone self without modification
// ParquetSource will override this
not_impl_err!("with_file_ordering_info not implemented for this FileSource")

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.

The comment doesn't match the implementation

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good catch @adriangb , fixed the comments in latest PR, thanks!

@zhuqi-lucas

Copy link
Copy Markdown
ContributorAuthor

Thank you @adriangb for review! I will wait for @alamb to double check before merging it, thanks!

@alamb

Copy link
Copy Markdown
Contributor

Let me know if you want me to double check anything specific. Otherwise don't feel like you have to wait for me -- I am not likely to be able to get back here for the next few days

BTW I also added this to the list of features to highlight for 52 n

@zhuqi-lucas
zhuqi-lucas added this pull request to the merge queueDec 17, 2025
Merged via the queue into apache:main with commit 3a41cc6Dec 17, 2025
14 of 15 checks passed
@zhuqi-lucas

Copy link
Copy Markdown
ContributorAuthor

Let me know if you want me to double check anything specific. Otherwise don't feel like you have to wait for me -- I am not likely to be able to get back here for the next few days

BTW I also added this to the list of features to highlight for 52 n

Thank you @alamb and @adriangb for the review and discussion! I've merged the PR now. If any issues come up, I'll address them promptly. Thanks again!

@adriangb

Copy link
Copy Markdown
Contributor

Thank you for this amazing piece of work! Very excited to see where this goes next.

@alamb

Copy link
Copy Markdown
Contributor

100% agree -- this is so cool

match sort_input.try_pushdown_sort(required_ordering)? {
SortOrderPushdownResult::Exact { inner } => {
// Data source guarantees perfect ordering - remove the Sort operator
Ok(Transformed::yes(inner))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the sort has fetch, can we directly remove the sort?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes @xudong963 , we will implement the remove sort part in phase 2, here is the ticket:
in #19329

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This PR implemented the inexact mode, which has similar performance from sorted data clickbench.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the sort operator will be removed in the enforce_sorting if the required ordering is matched.

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.

Yes agreed. I think what we ultimately need to do is return an ExecutionPlan that claims to satisfy the ordering and let the existing optimizer rule remove the sort for us.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, i was testing the exact reverse cases, the enforce_sorting is not enough for the exact reverse, but it's ok for current main branch.

github-merge-queueBot pushed a commit that referenced this pull request Dec 29, 2025
## Which issue does this PR close?
- Closes#19403
## Rationale for this change
After #19064 was completed, I found it couldn't meet our internal
project requirements:
1. **Redesign try_reverse_output using EquivalenceProperties**
- The previous implementation used a simple `is_reverse` method that
could only handle basic reverse matching
- Now leveraging `EquivalenceProperties` can handle more complex
scenarios, including constant column elimination and monotonic functions
2. **Switch to ordering_satisfy method for ordering matching**
- This method internally:
- Normalizes orderings (removes constant columns)
- Checks monotonic functions (like `date_trunc`, `CAST`, `CEIL`)
- Handles prefix matching
3. **Extend sort pushdown support to more operators**
- Added `try_pushdown_sort` implementation for `ProjectionExec`,
`FilterExec`, `CooperativeExec`
- These operators can now pass sort requests down to their children
## What changes are included in this PR?
### Core Changes:
1. **ParquetSource::try_reverse_output**
(datasource-parquet/src/source.rs)
- Added `eq_properties` parameter
- Reverses all orderings in equivalence properties
- Uses `ordering_satisfy` to check if reversed ordering satisfies the
request
- Removed `file_ordering` field and `with_file_ordering_info` method
2. **FileSource trait** (datasource/src/file.rs)
- Updated `try_reverse_output` signature with `eq_properties` parameter
- Added detailed documentation explaining parameter usage and examples
3. **FileScanConfig::try_pushdown_sort**
(datasource/src/file_scan_config.rs)
- Simplified logic to directly call `file_source.try_reverse_output`
- No longer needs to pre-check ordering satisfaction or set file
ordering info
4. **New operator support**
- `FilterExec::try_pushdown_sort` - Pushes sort below filters
- `ProjectionExec::try_pushdown_sort` - Pushes sort below projections
- `CooperativeExec::try_pushdown_sort` - Supports sort pushdown in
cooperative execution
5. **Removed obsolete methods**
- Deleted `LexOrdering::is_reverse` - replaced by `ordering_satisfy`
### Feature Enhancements:
**Supported optimization scenarios:**
1. **Constant column elimination** (Test 7)
```sql
-- File ordering: [timeframe ASC, period_end ASC]
-- Query: WHERE timeframe = 'quarterly' ORDER BY period_end DESC
-- Effect: After timeframe becomes constant, reverse scan is enabled
```
2. **Monotonic function support** (Test 8)
```sql
-- File ordering: [ts ASC]
-- Query: ORDER BY date_trunc('month', ts) DESC
-- Effect: date_trunc is monotonic, reverse scan satisfies the request
```
## Are these changes tested?
Yes, comprehensive tests have been added:
- **Test 7 (237 lines)**: Constant column elimination scenarios
- Single constant column filter
- Multi-value IN clauses (doesn't trigger optimization)
- Literal constants in sort expressions
- Non-leading column filters (edge cases)
- **Test 8 (355 lines)**: Monotonic function scenarios
- `date_trunc` (date truncation)
- `CAST` (type conversion)
- `CEIL` (ceiling)
- `ABS` (negative case - not monotonic over mixed positive/negative
range)
All tests verify:
- Presence of `reverse_row_groups=true` in physical plans
- Correctness of query results
## Are there any user-facing changes?
**API Changes:**
- `FileSource::try_reverse_output` signature changed (added
`eq_properties` parameter)
- Removed `FileSource::with_file_ordering_info` method
- Removed `LexOrdering::is_reverse` public method
**User-visible improvements:**
- More queries can leverage reverse row group scanning for optimization
- Especially queries with `WHERE` clauses that make certain columns
constant
- Queries using monotonic functions (like date functions, type
conversions)
**Note:** This PR returns `Inexact` results because only row group order
is reversed, not row order within row groups. Future enhancements could
include:
- File reordering based on statistics (returning `Exact`)
- Partial sort pushdown for prefix matches
---------
Co-authored-by: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commonRelated to common cratecoreCore DataFusion cratedatasourceChanges to the datasource cratedocumentationImprovements or additions to documentationexecutionRelated to the execution crateoptimizerOptimizer rulesphysical-exprChanges to the physical-expr cratesphysical-planChanges to the physical-plan crateprotoRelated to proto cratesqllogictestSQL Logic Tests (.slt)

Projects

None yet

7 participants

@zhuqi-lucas@adriangb@alamb@alamb-ghbot@martin-g@xudong963