alamb commented on code in PR #3912:
URL: https://github.com/apache/arrow-datafusion/pull/3912#discussion_r1002122290


##########
datafusion/physical-expr/src/expressions/binary.rs:
##########
@@ -771,32 +745,47 @@ fn compare_left_boundaries(
             // it is actually smaller than what we have right now) and it is a 
valid
             // value (e.g. [0, 100] < -100 would update the boundaries to [0, 
-100] if
             // there weren't the selectivity check).
-            if rhs_scalar_value < variadic_max && selectivity > 0.0 {
-                (variadic_min, rhs_scalar_value)
+            if right < left_max && selectivity > 0.0 {
+                (left_min, right)
             } else {
-                (variadic_min, variadic_max)
+                (left_min, left_max)
             }
         }
         Operator::Gt | Operator::GtEq => {
             // Same as above, but this time we want to limit the lower bound.
-            if rhs_scalar_value > variadic_min && selectivity > 0.0 {
-                (rhs_scalar_value, variadic_max)
+            if right > left_min && selectivity > 0.0 {
+                (right, left_max)
             } else {
-                (variadic_min, variadic_max)
+                (left_min, left_max)
             }
         }
         // For equality, we don't have the range problem so even if the 
selectivity
         // is 0.0, we can still update the boundaries.
-        Operator::Eq => (rhs_scalar_value.clone(), rhs_scalar_value),
+        Operator::Eq => (right.clone(), right),
         _ => unreachable!(),
     };
 
-    Some(ExprBoundaries {
-        min_value: new_min,
-        max_value: new_max,
-        distinct_count,
-        selectivity: Some(selectivity),
-    })
+    // The context represents all the knowledge we have gathered during the
+    // analysis process, which we can now add more since the expression's upper
+    // and lower boundaries might have changed.
+    let left_bounds = ExprBoundaries::new(left_min, left_max, 
left_bounds.distinct_count);
+    left.apply(context, &left_bounds);

Review Comment:
   🤔  don't the rules about combining information about the left and right side 
depend on the operators?
   
   Like if you have
   
   ```
   (a < 5) OR (a > 10)
   ```
   
   You would apply the boundaries differently than if it were a conjunction?
   
   ```
   (a < 5) AND (a > 10)
   ```
   



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