ozankabak commented on code in PR #14699:
URL: https://github.com/apache/datafusion/pull/14699#discussion_r1966455676
##########
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:
Possibly. Taking a look at other code in this module (e.g. `intersect`), I
see that they all use this pattern. Let's make an issue to investigate whether
we can refactor these methods to avoid allocations when the `self` object is
available for us to scavenge.
--
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
