geoffreyclaude commented on code in PR #26118:
URL: https://github.com/apache/datafusion/pull/26118#discussion_r4378986521


##########
datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs:
##########
@@ -1000,14 +976,24 @@ fn handle_custom_pushdown(
 ) -> Result<Option<Vec<Option<OrderingRequirements>>>> {
     let plan_children = plan.children();
 
-    // If the plan has no children, return early:
-    if plan_children.is_empty() {
+    // Return early for leaf nodes or fetched custom nodes.
+    // Pushing a sort below an unordered fetch changes which rows it keeps.
+    // Even an ordered fetch can select different rows if its input is 
reordered.
+    // Native limit and merge cases are handled earlier.
+    if plan_children.is_empty() || plan.fetch().is_some() {

Review Comment:
   Thanks, fixed. Refining the child sort is allowed again, while preserving 
all its original keys, including hidden ones. I also fixed limit propagation 
through custom offsets and added SQL coverage. `group_by.slt` passes again.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to