ahmetenis commented on code in PR #9125:
URL: https://github.com/apache/arrow-datafusion/pull/9125#discussion_r1482065702


##########
datafusion/core/src/physical_optimizer/enforce_sorting.rs:
##########
@@ -197,6 +202,41 @@ impl PhysicalOptimizerRule for EnforceSorting {
     }
 }
 
+fn replace_with_partial_sort(
+    plan: Arc<dyn ExecutionPlan>,
+) -> Result<Arc<dyn ExecutionPlan>> {
+    let plan_any = plan.as_any();
+    if let Some(sort_plan) = plan_any.downcast_ref::<SortExec>() {
+        let child = sort_plan.children()[0].clone();
+        if !unbounded_output(&child) {

Review Comment:
   I am not completely sure whether it will be safe to expand partial sort 
without incorporating the `ExternalSorter` used in `SortExec`. Would love to 
hear your thoughts on this.
   
   Also `SortExec` with unbounded input is already pipeline breaking, I wanted 
to first gate this change behind unbounded input and improve functionality 
without regressing the current behaviour.
   
   Opened an issue for expanding PartialSort to other use cases here: 
https://github.com/apache/arrow-datafusion/issues/9153



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

Reply via email to