mustafasrepo commented on code in PR #8107:
URL: https://github.com/apache/arrow-datafusion/pull/8107#discussion_r1388973882


##########
datafusion/physical-expr/src/equivalence.rs:
##########
@@ -869,9 +986,27 @@ impl EquivalenceProperties {
         self.ordering_satisfy_requirement(&sort_requirements)
     }
 
-    /// Checks whether the given sort requirements are satisfied by any of the
-    /// existing orderings.
+    /// Checks whether the given sort requirements are satisfied by any of the 
existing orderings.
+    /// This function applies an implicit projection to itself before calling 
`ordering_satisfy_requirement_helper`
+    /// to define the orderings of complex [`PhysicalExpr`]'s during analysis.
     pub fn ordering_satisfy_requirement(&self, reqs: LexRequirementRef) -> 
bool {
+        let exprs = reqs
+            .iter()
+            .map(|sort_req| sort_req.expr.clone())
+            .collect::<Vec<_>>();
+        let projection_mapping = self.implicit_projection_mapping(&exprs);
+        let projected_eqs =
+            self.project(&projection_mapping, 
projection_mapping.output_schema());
+        if let Some(projected_reqs) = 
projection_mapping.project_lex_reqs(reqs) {

Review Comment:
   > I am not quite sure I follow this logic -- does it say that any expression 
that can be projected maintains the ordering?
   No, it just projects everything as if they are evaluated (in terms of 
ordering properties). For instance if `ts` is not ordered. We know that 
`date_bin(ts)` is not ordered. Similarly `abs(x)` is not ordered no matter `x` 
is ordered or not. `update_ordering` considers the properties of functions.



##########
datafusion/physical-expr/src/equivalence.rs:
##########
@@ -869,9 +986,27 @@ impl EquivalenceProperties {
         self.ordering_satisfy_requirement(&sort_requirements)
     }
 
-    /// Checks whether the given sort requirements are satisfied by any of the
-    /// existing orderings.
+    /// Checks whether the given sort requirements are satisfied by any of the 
existing orderings.
+    /// This function applies an implicit projection to itself before calling 
`ordering_satisfy_requirement_helper`
+    /// to define the orderings of complex [`PhysicalExpr`]'s during analysis.
     pub fn ordering_satisfy_requirement(&self, reqs: LexRequirementRef) -> 
bool {
+        let exprs = reqs
+            .iter()
+            .map(|sort_req| sort_req.expr.clone())
+            .collect::<Vec<_>>();
+        let projection_mapping = self.implicit_projection_mapping(&exprs);
+        let projected_eqs =
+            self.project(&projection_mapping, 
projection_mapping.output_schema());
+        if let Some(projected_reqs) = 
projection_mapping.project_lex_reqs(reqs) {

Review Comment:
   > I am not quite sure I follow this logic -- does it say that any expression 
that can be projected maintains the ordering?
   
   No, it just projects everything as if they are evaluated (in terms of 
ordering properties). For instance if `ts` is not ordered. We know that 
`date_bin(ts)` is not ordered. Similarly `abs(x)` is not ordered no matter `x` 
is ordered or not. `update_ordering` considers the properties of functions.



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

Reply via email to