sgrebnov opened a new pull request, #23677:
URL: https://github.com/apache/datafusion/pull/23677
## Which issue does this PR close?
After the DataFusion 53 → 54 upgrade, queries with a `ScalarSubquery` stopped
respecting the global `ORDER BY` and returned unordered results.
## Rationale for this change
In add mode, `OutputRequirements` walks the plan via
`require_top_ordering_helper` to capture the global `ORDER BY` as an
`OutputRequirementExec`. The helper bails on any node with `children.len() !=
1`. `ScalarSubqueryExec` has multiple children (child 0 = order-transparent
main input, the rest = uncorrelated subquery plans),
so when it is the root the rule stops immediately and stamps an empty
`OutputRequirementExec(order_by=[], dist_by=Unspecified)` at the top — losing
the ordering requirement.
`ScalarSubqueryExec` is order-transparent on child 0
(`maintains_input_order()[0]== true`, no required input ordering), so the
search should descend through it like any other single-child order-preserving
operator.
## What changes are included in this PR?
`require_top_ordering_helper` now special-cases `ScalarSubqueryExec`: it
descends through child 0 to find and wrap the top `SortExec` /
`SortPreservingMergeExec`, reattaching the subquery children unchanged.
## Are these changes tested?
Yes — two tests in
`datafusion/core/tests/physical_optimizer/output_requirements.rs`:
1. **Rule level** (`require_top_ordering_descends_through_scalar_subquery`):
asserts the `OutputRequirementExec` carrying the ordering is placed *below* the
`ScalarSubqueryExec`; without the fix it lands empty at the root.
2. **End to end**
(`scalar_subquery_root_preserves_global_ordering_end_to_end`): runs the full
default optimizer pipeline over a `ScalarSubqueryExec` root whose ordering
comes from a `SortPreservingMergeExec` over a two-partition ordered source (the
shape federated/custom planners produce, with no `SortExec` backstop), then
executes the plan and checks the rows. Verified by toggling the fix:
| | executed output |
|---|---|
| **without fix** | `1, 3, 5, 7, 2, 4, 6, 8` — merge dropped, rows
partition-interleaved |
| **with fix** | `1, 2, 3, 4, 5, 6, 7, 8` — globally ordered |
Existing `subquery.slt` / TPC-H snapshots are unchanged. I was unable to
construct a SQL query that triggers the bug — over built-in sources the global
`ORDER BY` is always preserved regardless of the missing requirement. It only
surfaces when a custom physical planner supplies a plan shaped like the
end-to-end test: a `SortPreservingMergeExec` over already-sorted partitions,
with no `SortExec` above it.
## Are there any user-facing changes?
No.
--
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]
