ozankabak commented on code in PR #14699:
URL: https://github.com/apache/datafusion/pull/14699#discussion_r1958389834


##########
datafusion/expr-common/src/interval_arithmetic.rs:
##########
@@ -645,34 +651,78 @@ impl Interval {
         let upper = min_of_bounds(&self.upper, &rhs.upper);
 
         // New lower and upper bounds must always construct a valid interval.
-        assert!(
+        debug_assert!(
             (lower.is_null() || upper.is_null() || (lower <= upper)),
             "The intersection of two intervals can not be an invalid interval"
         );
 
         Ok(Some(Self { lower, upper }))
     }
 
-    /// Decide if this interval certainly contains, possibly contains, or can't
-    /// contain a [`ScalarValue`] (`other`) by returning `[true, true]`,
-    /// `[false, true]` or `[false, false]` respectively.
+    /// Compute the union of this interval with the given interval.
     ///
     /// NOTE: This function only works with intervals of the same data type.
     ///       Attempting to compare intervals of different data types will lead
     ///       to an error.
-    pub fn contains_value<T: Borrow<ScalarValue>>(&self, other: T) -> 
Result<bool> {
+    pub fn union<T: Borrow<Self>>(&self, other: T) -> Result<Self> {

Review Comment:
   A null bound represents unboundedness for its endpoint (and is treated that 
way throughout the library). Seems like we should improve the documentation for 
more clarity to make sure it is obvious.



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