jayhan94 opened a new pull request, #23744:
URL: https://github.com/apache/datafusion/pull/23744
## Which issue does this PR close?
- Closes #23534.
## Rationale for this change
`EnsureRequirements` may remove a `SortExec` when no ordering requirement is
visible from its parent.
This is normally valid, but not when the sorted input is consumed by a
limit. `LIMIT` and `OFFSET` use the input sequence to determine which rows are
returned. In the reported case, an intervening `ProjectionExec` removes the
sort key from its output schema, causing its `output_ordering()` to become
`None`. The optimizer consequently treats the sort below the projection as
unnecessary and removes it, producing incorrect query results.
`maintains_input_order()` only describes whether an operator reorders its
surviving rows, while `required_input_ordering()` requires a concrete ordering
expressible using the child schema. Neither property expresses that an operator
semantically depends on an existing input ordering.
## What changes are included in this PR?
- Add `ExecutionPlan::requires_input_order_preservation()`, with a default
value of `false` for each child.
- Make `GlobalLimitExec` and `LocalLimitExec` declare that their input
ordering must be preserved.
- Update sort enforcement to honor this property when deciding whether a
descendant `SortExec` can be removed.
- Propagate the preservation requirement through order-preserving operators
during sort pushdown.
- Prevent incompatible ordering requirements from being pushed through
order-sensitive operators, while still allowing compatible stricter orderings
and existing TopK optimizations.
- Add physical optimizer and SQL logic regression tests covering a
projection that hides the sort key below an `OFFSET`.
## Are these changes tested?
Yes.
The regression tests cover both the physical plan and the original SQL
behavior. Existing tests also verify that compatible sort pushdown and TopK
optimizations are preserved.
The following checks were run:
```shell
cargo test -p datafusion --test core_integration
physical_optimizer::enforce_sorting::
cargo test -p datafusion-sqllogictest --test sqllogictests -- subquery_sort
cargo test -p datafusion-physical-optimizer
cargo test -p datafusion-physical-plan --lib
cargo fmt --all -- --check
cargo clippy -p datafusion-physical-plan -p datafusion-physical-optimizer \
-p datafusion --all-targets --all-features --no-deps -- -D warnings
```
## Are there any user-facing changes?
Yes. Queries containing an ordered subquery followed by `LIMIT` or `OFFSET`
now preserve the ordering needed to select the correct rows, even when an
intervening projection hides the sort columns.
This PR also adds a defaulted method to the public `ExecutionPlan` trait.
Existing implementations do not need to be changed, and the change is not
breaking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
