viirya commented on code in PR #7885:
URL: https://github.com/apache/arrow-datafusion/pull/7885#discussion_r1367278071
##########
datafusion/physical-expr/src/intervals/interval_aritmetic.rs:
##########
@@ -1079,6 +1094,117 @@ mod tests {
Interval::make(lower, upper, (false, false))
}
+ #[test]
+ fn boolean_interval_test() -> Result<()> {
+ // Demonstrate / document how boolean Intervals work
+ #[derive(Debug)]
+ struct TestCase {
+ lower: bool,
+ upper: bool,
+ /// expected bounds when lower is open, upper is open
+ expected_open_open: (bool, bool),
+ /// expected bounds when lower is open, upper is closed
+ expected_open_closed: (bool, bool),
+ /// expected bounds when lower is closes, upper is open
+ expected_closed_open: (bool, bool),
+ // Not: closed/closed is the same as lower/upper
+ }
+
+ let cases = vec![
+ TestCase {
+ lower: false,
+ upper: false,
+ expected_open_open: (true, false), // whole range
+ expected_open_closed: (true, false),
+ expected_closed_open: (false, false),
+ },
+ TestCase {
+ lower: false,
+ upper: true,
+ expected_open_open: (true, false), // whole range
+ expected_open_closed: (true, true),
+ expected_closed_open: (false, false),
+ },
+ TestCase {
+ lower: true,
+ upper: false,
+ expected_open_open: (true, false), // whole range
+ expected_open_closed: (true, false),
+ expected_closed_open: (true, false),
+ },
+ TestCase {
+ lower: true,
+ upper: true,
+ expected_open_open: (true, false), // whole range
+ expected_open_closed: (true, true),
+ expected_closed_open: (true, false),
+ },
+ ];
+
+ // Asserts that the given interval has the expected (closed) bounds
+ fn assert_bool_interval(
+ name: &str,
+ interval: Interval,
+ expected_lower: bool,
+ expected_upper: bool,
+ ) {
+ let open = false;
+ let expected_interval = Interval {
+ // boolean bounds are always closed
+ lower: IntervalBound {
+ value: ScalarValue::Boolean(Some(expected_lower)),
+ open,
+ },
+ upper: IntervalBound {
+ value: ScalarValue::Boolean(Some(expected_upper)),
+ open,
+ },
+ };
+ assert_eq!(
+ interval, expected_interval,
+ "{name} failure\n\nActual: {interval:#?}\n\nExpected:
{expected_interval:#?}"
+ );
+ }
+
+ for case in cases {
+ println!("Case: {case:?}");
Review Comment:
```suggestion
```
##########
datafusion/physical-expr/src/intervals/interval_aritmetic.rs:
##########
@@ -1079,6 +1094,117 @@ mod tests {
Interval::make(lower, upper, (false, false))
}
+ #[test]
+ fn boolean_interval_test() -> Result<()> {
+ // Demonstrate / document how boolean Intervals work
+ #[derive(Debug)]
+ struct TestCase {
+ lower: bool,
+ upper: bool,
+ /// expected bounds when lower is open, upper is open
+ expected_open_open: (bool, bool),
+ /// expected bounds when lower is open, upper is closed
+ expected_open_closed: (bool, bool),
+ /// expected bounds when lower is closes, upper is open
+ expected_closed_open: (bool, bool),
+ // Not: closed/closed is the same as lower/upper
+ }
+
+ let cases = vec![
+ TestCase {
+ lower: false,
+ upper: false,
+ expected_open_open: (true, false), // whole range
+ expected_open_closed: (true, false),
+ expected_closed_open: (false, false),
+ },
+ TestCase {
+ lower: false,
+ upper: true,
+ expected_open_open: (true, false), // whole range
+ expected_open_closed: (true, true),
+ expected_closed_open: (false, false),
+ },
+ TestCase {
+ lower: true,
+ upper: false,
+ expected_open_open: (true, false), // whole range
+ expected_open_closed: (true, false),
+ expected_closed_open: (true, false),
+ },
+ TestCase {
+ lower: true,
+ upper: true,
+ expected_open_open: (true, false), // whole range
Review Comment:
An open `true` upper bound is mapped according to rule 2.
An open `true` lower bound is not mapped, right?
##########
datafusion/physical-expr/src/intervals/interval_aritmetic.rs:
##########
@@ -1079,6 +1094,117 @@ mod tests {
Interval::make(lower, upper, (false, false))
}
+ #[test]
+ fn boolean_interval_test() -> Result<()> {
+ // Demonstrate / document how boolean Intervals work
+ #[derive(Debug)]
+ struct TestCase {
+ lower: bool,
+ upper: bool,
+ /// expected bounds when lower is open, upper is open
+ expected_open_open: (bool, bool),
+ /// expected bounds when lower is open, upper is closed
+ expected_open_closed: (bool, bool),
+ /// expected bounds when lower is closes, upper is open
Review Comment:
```suggestion
/// expected bounds when lower is closed, upper is open
```
##########
datafusion/physical-expr/src/intervals/interval_aritmetic.rs:
##########
@@ -1079,6 +1094,117 @@ mod tests {
Interval::make(lower, upper, (false, false))
}
+ #[test]
+ fn boolean_interval_test() -> Result<()> {
+ // Demonstrate / document how boolean Intervals work
+ #[derive(Debug)]
+ struct TestCase {
+ lower: bool,
+ upper: bool,
+ /// expected bounds when lower is open, upper is open
+ expected_open_open: (bool, bool),
+ /// expected bounds when lower is open, upper is closed
+ expected_open_closed: (bool, bool),
+ /// expected bounds when lower is closes, upper is open
+ expected_closed_open: (bool, bool),
+ // Not: closed/closed is the same as lower/upper
+ }
+
+ let cases = vec![
+ TestCase {
+ lower: false,
+ upper: false,
+ expected_open_open: (true, false), // whole range
Review Comment:
An open `false` lower bound is mapped according to rule 1.
An open `false` upper bound is not mapped, right?
--
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]
