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

   ## Which issue does this PR close?
   
   - Closes #26646.
   
   ## Rationale for this change
   
   When a sorted input with more than one partition goes through 
`SortPreservingMergeExec`, the planner treats every ordering of the input as 
still holding after the merge. Only the merge key ordering actually holds. The 
other orderings are true within each partition, but not across them.
   
   Because of this, the planner can remove a sort that is still needed, and the 
query returns rows out of order. With two files that are each sorted by `a` and 
`b` and `target_partitions = 2`:
   
   ```sql
   SELECT * FROM (SELECT a, b FROM t ORDER BY a LIMIT 10) ORDER BY b;
   ```
   
   the plan has no sort for the outer `ORDER BY b`, and the rows come back as 
`(1, 10), (1, 1), (2, 20), (3, 2)`.
   
   ## What changes are included in this PR?
   
   - `SortPreservingMergeExec::compute_properties` clears the input orderings 
when there is more than one input partition, before adding the merge key 
ordering. This matches what `CoalescePartitionsExec` and `RepartitionExec` 
already do. With a single input partition nothing is merged, so the input 
orderings are kept as before.
   
   The preserve-order `RepartitionExec` case mentioned in the issue is not 
covered here.
   
   Draft #26643 moves this logic into `EquivalenceProperties::merged`. If that 
lands first, I'll update this PR to use `.merged(input_partitions, false)` 
followed by `add_ordering`.
   
   ## What is the testing strategy for this PR?
   
   - New sqllogictest case at the end of `order.slt`, using the reproduction 
from the issue. It checks both the query result and the physical plan, which 
must keep the `SortExec` above the `SortPreservingMergeExec`. It fails on 
`main` with the wrong row order from the issue, and passes with this change.
   - The existing sqllogictests and the `datafusion-physical-plan` / 
`datafusion-physical-optimizer` unit tests pass with no changes to expected 
plans.
   
   ## Are there any user-facing changes?
   
   Queries like the one above now return correctly ordered results. No public 
API changes.


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