EeshanBembi commented on code in PR #16905:
URL: https://github.com/apache/datafusion/pull/16905#discussion_r2238068076
##########
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're right that architecturally it should belong in EnforceSorting - I can
see the logic there. The issue is I'm running into implementation challenges
trying to make it work cleanly within that framework, and I've hit regressions
in about 20% of test cases that I'm still debugging.
I'm currently investigating those regressions to understand what's causing
them before moving forward. Once I've resolved those issues, I'll raise the PR
and we can iterate on the approach.
Thanks for the collaboration on this - having another perspective on the
architecture is helpful.
--
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
