[GitHub] [arrow-datafusion] isidentical commented on a diff in pull request #3912: [wip]: Expression boundary analysis framework

[GitBox <gi...@apache.org>]

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


##########
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:
   @alamb this is where we use `apply` (to update the current context with the `left`'s new lower/upper bounds)



-- 
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-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org