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


##########
datafusion/physical-expr/src/equivalence.rs:
##########
@@ -131,17 +210,120 @@ impl<T: Eq + Clone + Hash> EquivalenceProperties<T> {
 /// where both `a ASC` and `b DESC` can describe the table ordering. With
 /// `OrderingEquivalenceProperties`, we can keep track of these equivalences
 /// and treat `a ASC` and `b DESC` as the same ordering requirement.
-pub type OrderingEquivalenceProperties = EquivalenceProperties<LexOrdering>;
+#[derive(Debug, Clone)]
+pub struct OrderingEquivalenceProperties {
+    oeq_class: Option<OrderingEquivalentClass>,
+    /// Keeps track of expressions that have constant value.
+    constants: Vec<Arc<dyn PhysicalExpr>>,
+    schema: SchemaRef,
+}
 
 impl OrderingEquivalenceProperties {
+    /// Create an empty `OrderingEquivalenceProperties`
+    pub fn new(schema: SchemaRef) -> Self {
+        Self {
+            oeq_class: None,
+            constants: vec![],
+            schema,
+        }
+    }
+
+    /// Extends `OrderingEquivalenceProperties` by adding ordering inside the 
`other`
+    /// to the `self.oeq_class`.
+    pub fn extend(&mut self, other: Option<OrderingEquivalentClass>) {
+        if let Some(other) = other {
+            if let Some(class) = &mut self.oeq_class {
+                class.others.insert(other.head);
+                class.others.extend(other.others);
+            } else {
+                self.oeq_class = Some(other);
+            }
+        }
+    }
+
+    pub fn oeq_class(&self) -> Option<&OrderingEquivalentClass> {
+        self.oeq_class.as_ref()
+    }
+
+    /// Adds new equal conditions into the EquivalenceProperties. New equal
+    /// conditions usually come from equality predicates in a join/filter.
+    pub fn add_equal_conditions(&mut self, new_conditions: (&LexOrdering, 
&LexOrdering)) {
+        if let Some(class) = &mut self.oeq_class {
+            class.insert(new_conditions.0.clone());
+            class.insert(new_conditions.1.clone());
+        } else {
+            let head = new_conditions.0.clone();
+            let others = vec![new_conditions.1.clone()];
+            self.oeq_class = Some(OrderingEquivalentClass::new(head, others))
+        }
+    }
+
+    /// Add physical expression that have constant value to the 
`self.constants`
+    pub fn with_constants(mut self, constants: Vec<Arc<dyn PhysicalExpr>>) -> 
Self {
+        constants.into_iter().for_each(|constant| {
+            if !physical_exprs_contains(&self.constants, &constant) {
+                self.constants.push(constant);
+            }
+        });
+        self
+    }
+
+    pub fn schema(&self) -> SchemaRef {
+        self.schema.clone()
+    }
+
+    /// This function normalizes `sort_reqs` by
+    /// - removing expressions that have constant value from requirement
+    /// - replacing sections that are in the `self.oeq_class.others` with 
`self.oeq_class.head`
+    /// - removing sections that satisfies global ordering that are in the 
post fix of requirement
+    pub fn normalize_sort_requirements(
+        &self,
+        sort_reqs: &[PhysicalSortRequirement],
+    ) -> Vec<PhysicalSortRequirement> {
+        let normalized_sort_reqs =
+            prune_sort_reqs_with_constants(sort_reqs, &self.constants);
+        let mut normalized_sort_reqs = collapse_lex_req(normalized_sort_reqs);
+        if let Some(oeq_class) = &self.oeq_class {
+            for item in oeq_class.others() {
+                let item = PhysicalSortRequirement::from_sort_exprs(item);
+                let item = prune_sort_reqs_with_constants(&item, 
&self.constants);
+                let ranges = get_compatible_ranges(&normalized_sort_reqs, 
&item);
+                let mut offset: i64 = 0;

Review Comment:
   `offset += head.len() as i64 - range as i64;` can result in negative number, 
hence we use `i64`. However, I agree that casting to `i64` and `usize` a bit 
weird. If we can avoid it, it would be great. I will try to re-write this 
logic, to remove this casting.



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