wiedld commented on code in PR #14821:
URL: https://github.com/apache/datafusion/pull/14821#discussion_r1972157836


##########
datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs:
##########
@@ -98,66 +98,91 @@ fn pushdown_sorts_helper(
         .ordering_satisfy_requirement(&parent_reqs);
 
     if is_sort(plan) {
-        let sort_fetch = plan.fetch();
-        let required_ordering = plan
+        let current_sort_fetch = plan.fetch();
+        let parent_req_fetch = requirements.data.fetch;
+
+        let child_reqs = plan
             .output_ordering()
             .cloned()
             .map(LexRequirement::from)
             .unwrap_or_default();
-        if !satisfy_parent {
-            // Make sure this `SortExec` satisfies parent requirements:
-            let sort_reqs = 
requirements.data.ordering_requirement.unwrap_or_default();
-            // It's possible current plan (`SortExec`) has a fetch value.
-            // And if both of them have fetch values, we should use the 
minimum one.
-            if let Some(fetch) = sort_fetch {
-                if let Some(requirement_fetch) = requirements.data.fetch {
-                    requirements.data.fetch = 
Some(fetch.min(requirement_fetch));
-                }
-            }
-            let fetch = requirements.data.fetch.or(sort_fetch);
+        let parent_is_stricter = plan
+            .equivalence_properties()
+            .requirements_compatible(&parent_reqs, &child_reqs);

Review Comment:
   The requirements_compatible ([docs 
here](https://github.com/apache/datafusion/blob/f5b7affecd90e9be26289d869c4a542359cb98e3/datafusion/physical-expr/src/equivalence/properties.rs#L679-L681))
 determine if the set of requirements is equal or stricter than the other set 
of requirements.
   
   On current main, the [handling of sort 
nodes](https://github.com/apache/datafusion/blob/ea0686b8ce4b45cb61ba7fecac563cfa17f13595/datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs#L100-L149)
 includes the use of 
[pushdown_requirement_to_children](https://github.com/apache/datafusion/blob/ea0686b8ce4b45cb61ba7fecac563cfa17f13595/datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs#L125)
 which itself uses the 
[requirements_compatible](https://github.com/apache/datafusion/blob/ea0686b8ce4b45cb61ba7fecac563cfa17f13595/datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs#L221).
 I simply removed some of the misdirection -- but I could add it back.



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