Skip to content

Commit a3a5ca4

Browse files
authored
Fix misleading boolean 'null' interval tests (#18620)
## Which issue does this PR close? - None, simply corrects two unit tests ## Rationale for this change The `test_and_null_boolean_intervals` and `test_or_null_boolean_intervals` are a bit misleading. They try to create a 'null' interval using `Interval::try_new(ScalarValue::Boolean(None), ScalarValue::Boolean(None))`. The implementation of `try_new` normalises this to `Interval::UNCERTAIN`, which is not the same as 'null'. ## What changes are included in this PR? Adds tests demonstrating: - `Interval::try_new(ScalarValue::Boolean(None), ScalarValue::Boolean(None)) == Interval::UNCERTAIN` - `Interval::UNCERTAIN` contains `ScalarValue::Boolean(Some(true))` and `ScalarValue::Boolean(Some(false))` - `Interval::UNCERTAIN` does not contain `ScalarValue::Boolean(None)` or `ScalarValue::Null` Renames `test_and_null_boolean_intervals` and `test_and_null_boolean_intervals` ## Are these changes tested? Test only changes ## Are there any user-facing changes? No
1 parent 65bd13d commit a3a5ca4

File tree

1 file changed

+39
-16
lines changed

1 file changed

+39
-16
lines changed

datafusion/expr-common/src/interval_arithmetic.rs

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2533,47 +2533,70 @@ mod tests {
25332533
Ok(())
25342534
}
25352535

2536+
// Tests that there's no such thing as a 'null' boolean interval.
2537+
// An interval with two `Boolean(None)` boundaries is normalised to `Interval::UNCERTAIN`.
25362538
#[test]
2537-
fn test_and_null_boolean_intervals() -> Result<()> {
2539+
fn test_null_boolean_interval() {
25382540
let null_interval =
2539-
Interval::try_new(ScalarValue::Boolean(None), ScalarValue::Boolean(None))?;
2541+
Interval::try_new(ScalarValue::Boolean(None), ScalarValue::Boolean(None))
2542+
.unwrap();
2543+
2544+
assert_eq!(null_interval, Interval::UNCERTAIN);
2545+
}
2546+
2547+
// Asserts that `Interval::UNCERTAIN` represents a set that contains `true`, `false`, and does
2548+
// not contain `null`.
2549+
#[test]
2550+
fn test_uncertain_boolean_interval() {
2551+
assert!(Interval::UNCERTAIN
2552+
.contains_value(ScalarValue::Boolean(Some(true)))
2553+
.unwrap());
2554+
assert!(Interval::UNCERTAIN
2555+
.contains_value(ScalarValue::Boolean(Some(false)))
2556+
.unwrap());
2557+
assert!(!Interval::UNCERTAIN
2558+
.contains_value(ScalarValue::Boolean(None))
2559+
.unwrap());
2560+
assert!(!Interval::UNCERTAIN
2561+
.contains_value(ScalarValue::Null)
2562+
.unwrap());
2563+
}
25402564

2541-
let and_result = null_interval.and(&Interval::CERTAINLY_FALSE)?;
2565+
#[test]
2566+
fn test_and_uncertain_boolean_intervals() -> Result<()> {
2567+
let and_result = Interval::UNCERTAIN.and(&Interval::CERTAINLY_FALSE)?;
25422568
assert_eq!(and_result, Interval::CERTAINLY_FALSE);
25432569

2544-
let and_result = Interval::CERTAINLY_FALSE.and(&null_interval)?;
2570+
let and_result = Interval::CERTAINLY_FALSE.and(&Interval::UNCERTAIN)?;
25452571
assert_eq!(and_result, Interval::CERTAINLY_FALSE);
25462572

2547-
let and_result = null_interval.and(&Interval::CERTAINLY_TRUE)?;
2573+
let and_result = Interval::UNCERTAIN.and(&Interval::CERTAINLY_TRUE)?;
25482574
assert_eq!(and_result, Interval::UNCERTAIN);
25492575

2550-
let and_result = Interval::CERTAINLY_TRUE.and(&null_interval)?;
2576+
let and_result = Interval::CERTAINLY_TRUE.and(&Interval::UNCERTAIN)?;
25512577
assert_eq!(and_result, Interval::UNCERTAIN);
25522578

2553-
let and_result = null_interval.and(&null_interval)?;
2579+
let and_result = Interval::UNCERTAIN.and(&Interval::UNCERTAIN)?;
25542580
assert_eq!(and_result, Interval::UNCERTAIN);
25552581

25562582
Ok(())
25572583
}
25582584

25592585
#[test]
2560-
fn test_or_null_boolean_intervals() -> Result<()> {
2561-
let null_interval =
2562-
Interval::try_new(ScalarValue::Boolean(None), ScalarValue::Boolean(None))?;
2563-
2564-
let or_result = null_interval.or(&Interval::CERTAINLY_FALSE)?;
2586+
fn test_or_uncertain_boolean_intervals() -> Result<()> {
2587+
let or_result = Interval::UNCERTAIN.or(&Interval::CERTAINLY_FALSE)?;
25652588
assert_eq!(or_result, Interval::UNCERTAIN);
25662589

2567-
let or_result = Interval::CERTAINLY_FALSE.or(&null_interval)?;
2590+
let or_result = Interval::CERTAINLY_FALSE.or(&Interval::UNCERTAIN)?;
25682591
assert_eq!(or_result, Interval::UNCERTAIN);
25692592

2570-
let or_result = null_interval.or(&Interval::CERTAINLY_TRUE)?;
2593+
let or_result = Interval::UNCERTAIN.or(&Interval::CERTAINLY_TRUE)?;
25712594
assert_eq!(or_result, Interval::CERTAINLY_TRUE);
25722595

2573-
let or_result = Interval::CERTAINLY_TRUE.or(&null_interval)?;
2596+
let or_result = Interval::CERTAINLY_TRUE.or(&Interval::UNCERTAIN)?;
25742597
assert_eq!(or_result, Interval::CERTAINLY_TRUE);
25752598

2576-
let or_result = null_interval.or(&null_interval)?;
2599+
let or_result = Interval::UNCERTAIN.or(&Interval::UNCERTAIN)?;
25772600
assert_eq!(or_result, Interval::UNCERTAIN);
25782601

25792602
Ok(())

0 commit comments

Comments
 (0)