You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "HarshilMital (via GitHub)" <gi...@apache.org> on 2026/10/23 09:03:29 UTC

[PR] fix: SortPreservingMergeExec drops input orderings when merging partitions [datafusion]

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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] fix: SortPreservingMergeExec drops input orderings when merging partitions [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 merged PR #26654:
URL: https://github.com/apache/datafusion/pull/26654


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org