jayzhan211 commented on code in PR #26459:
URL: https://github.com/apache/datafusion/pull/26459#discussion_r4324940273


##########
datafusion/physical-optimizer/src/ensure_requirements/mod.rs:
##########
@@ -267,10 +279,15 @@ impl PhysicalOptimizerRule for EnsureRequirements {
         assign_initial_requirements(&mut sort_pushdown);
         let adjusted = pushdown_sorts(sort_pushdown)?;
 
-        // 3d: Partial sort
-        adjusted
+        // 3d: Sort where an ordering requirement cannot be proven

Review Comment:
   Copying the constant in `try_with_offset` fixes both queries, and the plan 
no longer needs a sort on `k` at all. One caveat: for Left and Full joins, 
unmatched rows pad the right side with NULLs, so its constants have to be 
cleared there, or a sort that is needed gets dropped. There is a test for that 
case now. The PR now fixes the join instead of adding the EnsureRequirements 
step.
   
   The left side has the same problem for Right and Full joins: its constants 
survive the NULL padding and can give wrongly ordered results. #26042 already 
rebuilds the classes of the side padded with NULLs, which covers that, so I 
left the left side alone here and noted it in the description.
   



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