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