berkaysynnada commented on code in PR #16905:
URL: https://github.com/apache/datafusion/pull/16905#discussion_r2234793079


##########
datafusion/core/src/physical_planner.rs:
##########
@@ -886,6 +887,26 @@ impl DefaultPhysicalPlanner {
                         "SortExec requires at least one sort expression"
                     );
                 };
+
+                // Check if we can use PartialSortExec instead of SortExec

Review Comment:
   You might be right, but during the enforce sorting step, we’re working with 
a more established and well-defined plan. I don’t think it has fewer 
capabilities than what we have at the logical planning stage.
   
   I’m not opposed to detecting partial sort conditions and inserting them 
during logical to physical mapping, but the checks should remain simple. We 
should avoid introducing too much optimization complexity at that stage.
   
   So here’s what I propose: go ahead and prepare this PR the way you envision 
it, and once it’s ready, I’d like to take a pass at it myself and experiment a 
bit. WDYT? 



-- 
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-unsubscr...@datafusion.apache.org

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


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

Reply via email to