sgrebnov opened a new pull request, #23146:
URL: https://github.com/apache/datafusion/pull/23146

   > **Draft / WIP.** Opening early for visibility. The defect is currently 
covered
   > by a rule-level unit test; I'm still working on additional visible 
(`EXPLAIN`)
   > and `.slt`-based coverage on top of it (see *Are these changes tested?*).
   
   ## Which issue does this PR close?
   
   - _TODO (draft): file / link a tracking issue._
   
   ## Rationale for this change
   
   `OutputRequirements` (add mode) walks the plan via 
`require_top_ordering_helper`
   to capture the query's global `ORDER BY` as an `OutputRequirementExec`. The
   helper bails on any node with more than one child (`children.len() != 1`).
   `ScalarSubqueryExec` is such a node — child 0 is the order-transparent main 
input
   and the rest are uncorrelated subquery plans — so when it is the plan root 
the
   rule stops immediately and stamps an empty
   `OutputRequirementExec(order_by=[], dist_by=Unspecified)` at the top, never
   recording the global ordering requirement.
   
   `ScalarSubqueryExec` is order-transparent on its main input
   (`maintains_input_order()[0] == true`, no required input ordering), so the 
search
   should descend through child 0, as it does for other single-child
   order-preserving operators.
   
   ## 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`, reattaching the subquery
   children unchanged.
   
   ## Are these changes tested?
   
   A new unit test (`add_mode_descends_through_scalar_subquery` in
   `datafusion/core/tests/physical_optimizer/output_requirements.rs`) asserts 
the
   `OutputRequirementExec` carrying the real ordering is placed *below* the
   `ScalarSubqueryExec`; without the fix the rule produces the empty
   `order_by=[], dist_by=Unspecified` requirement at the root.
   
   Existing `subquery.slt` / TPC-H plan snapshots are unchanged: for built-in
   sources `EnforceSorting` independently rebuilds the order-preserving merge, 
so
   final plans are identical. **WIP:** adding visible/end-to-end coverage on 
top of
   the unit test.
   
   ## 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]

Reply via email to