Skip to content

Commit bd30fe2

Browse files
pepijnvepepijnve
authored
Rename boolean Interval constants to match NullableInterval (apache#18654)
## Which issue does this PR close? - None, followup to apache#18625 ## Rationale for this change The discussion in apache#18625 made it clear that the naming of the truth value intervals would benefit from consistency. After some back and forth we concluded that consistency with the SQL specification seemed like the best idea. This PR renames the boolean `Interval` and `NullableInterval` constants to all be consistent. ## What changes are included in this PR? Mechanical refactoring where the following renames were performed: - `Interval::CERTAINLY_TRUE` -> `Interval::TRUE` - `Interval::CERTAINLY_FALSE` -> `Interval::FALSE` - `Interval::UNCERTAIN` -> `Interval::TRUE_OR_FALSE` - `NullableInterval::UNCERTAIN` -> `NullableInterval::ANY_TRUTH_VALUE` The old constant names have been retained with a deprecation note. ## Are these changes tested? No new code ## Are there any user-facing changes? Yes, the old constants are now marked deprecated
1 parent e4bc514 commit bd30fe2

11 files changed

Lines changed: 343 additions & 811 deletions

File tree

‎datafusion/expr-common/src/interval_arithmetic.rs‎

Lines changed: 257 additions & 692 deletions
Large diffs are not rendered by default.

‎datafusion/expr-common/src/statistics.rs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,9 @@ impl Distribution {
162162
/// - A [`Uniform`] distribution's range is simply its interval.
163163
/// - An [`Exponential`] distribution's range is `[offset, +∞)`.
164164
/// - A [`Gaussian`] distribution's range is unbounded.
165-
/// - A [`Bernoulli`] distribution's range is [`Interval::UNCERTAIN`], if
166-
/// `p` is neither `0` nor `1`. Otherwise, it is [`Interval::CERTAINLY_FALSE`]
167-
/// and [`Interval::CERTAINLY_TRUE`], respectively.
165+
/// - A [`Bernoulli`] distribution's range is [`Interval::TRUE_OR_FALSE`], if
166+
/// `p` is neither `0` nor `1`. Otherwise, it is [`Interval::FALSE`]
167+
/// and [`Interval::TRUE`], respectively.
168168
/// - A [`Generic`] distribution is unbounded by default, but more information
169169
/// may be present.
170170
pub fn range(&self) -> Result<Interval> {
@@ -519,11 +519,11 @@ impl BernoulliDistribution {
519519
// Unwraps are safe as the constructor guarantees that the data type
520520
// supports zero and one values.
521521
if ScalarValue::new_zero(&dt).unwrap().eq(&self.p) {
522-
Interval::CERTAINLY_FALSE
522+
Interval::FALSE
523523
} else if ScalarValue::new_one(&dt).unwrap().eq(&self.p) {
524-
Interval::CERTAINLY_TRUE
524+
Interval::TRUE
525525
} else {
526-
Interval::UNCERTAIN
526+
Interval::TRUE_OR_FALSE
527527
}
528528
}
529529
}
@@ -736,11 +736,11 @@ pub fn create_bernoulli_from_comparison(
736736
}
737737
let (li, ri) = (left.range()?, right.range()?);
738738
let range_evaluation = apply_operator(op, &li, &ri)?;
739-
if range_evaluation.eq(&Interval::CERTAINLY_FALSE) {
739+
if range_evaluation.eq(&Interval::FALSE) {
740740
Distribution::new_bernoulli(ScalarValue::from(0.0))
741-
} else if range_evaluation.eq(&Interval::CERTAINLY_TRUE) {
741+
} else if range_evaluation.eq(&Interval::TRUE) {
742742
Distribution::new_bernoulli(ScalarValue::from(1.0))
743-
} else if range_evaluation.eq(&Interval::UNCERTAIN) {
743+
} else if range_evaluation.eq(&Interval::TRUE_OR_FALSE) {
744744
Distribution::new_bernoulli(ScalarValue::try_from(&DataType::Float64)?)
745745
} else {
746746
internal_err!("This function must be called with a comparison operator")
@@ -897,7 +897,7 @@ mod tests {
897897
})
898898
);
899899

900-
assert!(Distribution::new_uniform(Interval::UNCERTAIN).is_err());
900+
assert!(Distribution::new_uniform(Interval::TRUE_OR_FALSE).is_err());
901901
Ok(())
902902
}
903903

@@ -1010,7 +1010,7 @@ mod tests {
10101010
ScalarValue::Null,
10111011
ScalarValue::Null,
10121012
ScalarValue::Null,
1013-
Interval::UNCERTAIN,
1013+
Interval::TRUE_OR_FALSE,
10141014
),
10151015
false,
10161016
),

‎datafusion/functions/src/math/abs.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,9 @@ impl ScalarUDFImpl for AbsFunc {
175175
let range = &arg.range;
176176
let zero_point = Interval::make_zero(&range.lower().data_type())?;
177177

178-
if range.gt_eq(&zero_point)? == Interval::CERTAINLY_TRUE {
178+
if range.gt_eq(&zero_point)? == Interval::TRUE {
179179
Ok(arg.sort_properties)
180-
} else if range.lt_eq(&zero_point)? == Interval::CERTAINLY_TRUE {
180+
} else if range.lt_eq(&zero_point)? == Interval::TRUE {
181181
Ok(-arg.sort_properties)
182182
} else {
183183
Ok(SortProperties::Unordered)

‎datafusion/functions/src/math/monotonicity.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ pub fn acos_order(input: &[ExprProperties]) -> Result<SortProperties> {
3131
let valid_domain =
3232
Interval::make_symmetric_unit_interval(&range.lower().data_type())?;
3333

34-
if valid_domain.contains(range)? == Interval::CERTAINLY_TRUE {
34+
if valid_domain.contains(range)? == Interval::TRUE {
3535
Ok(-arg.sort_properties)
3636
} else {
3737
exec_err!("Input range of ACOS contains out-of-domain values")
@@ -72,7 +72,7 @@ pub fn acosh_order(input: &[ExprProperties]) -> Result<SortProperties> {
7272
ScalarValue::try_from(&range.upper().data_type())?,
7373
)?;
7474

75-
if valid_domain.contains(range)? == Interval::CERTAINLY_TRUE {
75+
if valid_domain.contains(range)? == Interval::TRUE {
7676
Ok(arg.sort_properties)
7777
} else {
7878
exec_err!("Input range of ACOSH contains out-of-domain values")
@@ -110,7 +110,7 @@ pub fn asin_order(input: &[ExprProperties]) -> Result<SortProperties> {
110110
let valid_domain =
111111
Interval::make_symmetric_unit_interval(&range.lower().data_type())?;
112112

113-
if valid_domain.contains(range)? == Interval::CERTAINLY_TRUE {
113+
if valid_domain.contains(range)? == Interval::TRUE {
114114
Ok(arg.sort_properties)
115115
} else {
116116
exec_err!("Input range of ASIN contains out-of-domain values")
@@ -207,7 +207,7 @@ pub fn atanh_order(input: &[ExprProperties]) -> Result<SortProperties> {
207207
let valid_domain =
208208
Interval::make_symmetric_unit_interval(&range.lower().data_type())?;
209209

210-
if valid_domain.contains(range)? == Interval::CERTAINLY_TRUE {
210+
if valid_domain.contains(range)? == Interval::TRUE {
211211
Ok(arg.sort_properties)
212212
} else {
213213
exec_err!("Input range of ATANH contains out-of-domain values")
@@ -371,9 +371,9 @@ pub fn cosh_order(input: &[ExprProperties]) -> Result<SortProperties> {
371371

372372
let zero_point = Interval::make_zero(&range.lower().data_type())?;
373373

374-
if range.gt_eq(&zero_point)? == Interval::CERTAINLY_TRUE {
374+
if range.gt_eq(&zero_point)? == Interval::TRUE {
375375
Ok(arg.sort_properties)
376-
} else if range.lt_eq(&zero_point)? == Interval::CERTAINLY_TRUE {
376+
} else if range.lt_eq(&zero_point)? == Interval::TRUE {
377377
Ok(-arg.sort_properties)
378378
} else {
379379
Ok(SortProperties::Unordered)
@@ -498,7 +498,7 @@ pub fn ln_order(input: &[ExprProperties]) -> Result<SortProperties> {
498498

499499
let zero_point = Interval::make_zero(&range.lower().data_type())?;
500500

501-
if range.gt_eq(&zero_point)? == Interval::CERTAINLY_TRUE {
501+
if range.gt_eq(&zero_point)? == Interval::TRUE {
502502
Ok(arg.sort_properties)
503503
} else {
504504
exec_err!("Input range of LN contains out-of-domain values")
@@ -536,7 +536,7 @@ pub fn log2_order(input: &[ExprProperties]) -> Result<SortProperties> {
536536

537537
let zero_point = Interval::make_zero(&range.lower().data_type())?;
538538

539-
if range.gt_eq(&zero_point)? == Interval::CERTAINLY_TRUE {
539+
if range.gt_eq(&zero_point)? == Interval::TRUE {
540540
Ok(arg.sort_properties)
541541
} else {
542542
exec_err!("Input range of LOG2 contains out-of-domain values")
@@ -574,7 +574,7 @@ pub fn log10_order(input: &[ExprProperties]) -> Result<SortProperties> {
574574

575575
let zero_point = Interval::make_zero(&range.lower().data_type())?;
576576

577-
if range.gt_eq(&zero_point)? == Interval::CERTAINLY_TRUE {
577+
if range.gt_eq(&zero_point)? == Interval::TRUE {
578578
Ok(arg.sort_properties)
579579
} else {
580580
exec_err!("Input range of LOG10 contains out-of-domain values")
@@ -701,7 +701,7 @@ pub fn sqrt_order(input: &[ExprProperties]) -> Result<SortProperties> {
701701

702702
let zero_point = Interval::make_zero(&range.lower().data_type())?;
703703

704-
if range.gt_eq(&zero_point)? == Interval::CERTAINLY_TRUE {
704+
if range.gt_eq(&zero_point)? == Interval::TRUE {
705705
Ok(arg.sort_properties)
706706
} else {
707707
exec_err!("Input range of SQRT contains out-of-domain values")

‎datafusion/physical-expr-common/src/physical_expr.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,9 @@ pub trait PhysicalExpr: Any + Send + Sync + Display + Debug + DynEq + DynHash {
250250
let output_interval = self.evaluate_bounds(children_ranges_refs.as_slice())?;
251251
let dt = output_interval.data_type();
252252
if dt.eq(&DataType::Boolean) {
253-
let p = if output_interval.eq(&Interval::CERTAINLY_TRUE) {
253+
let p = if output_interval.eq(&Interval::TRUE) {
254254
ScalarValue::new_one(&dt)
255-
} else if output_interval.eq(&Interval::CERTAINLY_FALSE) {
255+
} else if output_interval.eq(&Interval::FALSE) {
256256
ScalarValue::new_zero(&dt)
257257
} else {
258258
ScalarValue::try_from(&dt)
@@ -312,9 +312,9 @@ pub trait PhysicalExpr: Any + Send + Sync + Display + Debug + DynEq + DynHash {
312312
Ok((*child).clone())
313313
} else if new_interval.data_type().eq(&DataType::Boolean) {
314314
let dt = old_interval.data_type();
315-
let p = if new_interval.eq(&Interval::CERTAINLY_TRUE) {
315+
let p = if new_interval.eq(&Interval::TRUE) {
316316
ScalarValue::new_one(&dt)
317-
} else if new_interval.eq(&Interval::CERTAINLY_FALSE) {
317+
} else if new_interval.eq(&Interval::FALSE) {
318318
ScalarValue::new_zero(&dt)
319319
} else {
320320
unreachable!("Given that we have a range reduction for a boolean interval, we should have certainty")

‎datafusion/physical-expr/src/analysis.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,7 @@ pub fn analyze(
212212
}
213213
}
214214

215-
match graph
216-
.update_ranges(&mut target_indices_and_boundaries, Interval::CERTAINLY_TRUE)?
217-
{
215+
match graph.update_ranges(&mut target_indices_and_boundaries, Interval::TRUE)? {
218216
PropagationResult::Success => {
219217
shrink_boundaries(&graph, target_boundaries, &target_expr_and_indices)
220218
}

‎datafusion/physical-expr/src/expressions/binary.rs‎

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -333,33 +333,27 @@ impl PhysicalExpr for BinaryExpr {
333333
let right_interval = children[1];
334334

335335
if self.op.eq(&Operator::And) {
336-
if interval.eq(&Interval::CERTAINLY_TRUE) {
336+
if interval.eq(&Interval::TRUE) {
337337
// A certainly true logical conjunction can only derive from possibly
338338
// true operands. Otherwise, we prove infeasibility.
339-
Ok((!left_interval.eq(&Interval::CERTAINLY_FALSE)
340-
&& !right_interval.eq(&Interval::CERTAINLY_FALSE))
341-
.then(|| vec![Interval::CERTAINLY_TRUE, Interval::CERTAINLY_TRUE]))
342-
} else if interval.eq(&Interval::CERTAINLY_FALSE) {
339+
Ok((!left_interval.eq(&Interval::FALSE)
340+
&& !right_interval.eq(&Interval::FALSE))
341+
.then(|| vec![Interval::TRUE, Interval::TRUE]))
342+
} else if interval.eq(&Interval::FALSE) {
343343
// If the logical conjunction is certainly false, one of the
344344
// operands must be false. However, it's not always possible to
345345
// determine which operand is false, leading to different scenarios.
346346

347347
// If one operand is certainly true and the other one is uncertain,
348348
// then the latter must be certainly false.
349-
if left_interval.eq(&Interval::CERTAINLY_TRUE)
350-
&& right_interval.eq(&Interval::UNCERTAIN)
349+
if left_interval.eq(&Interval::TRUE)
350+
&& right_interval.eq(&Interval::TRUE_OR_FALSE)
351351
{
352-
Ok(Some(vec![
353-
Interval::CERTAINLY_TRUE,
354-
Interval::CERTAINLY_FALSE,
355-
]))
356-
} else if right_interval.eq(&Interval::CERTAINLY_TRUE)
357-
&& left_interval.eq(&Interval::UNCERTAIN)
352+
Ok(Some(vec![Interval::TRUE, Interval::FALSE]))
353+
} else if right_interval.eq(&Interval::TRUE)
354+
&& left_interval.eq(&Interval::TRUE_OR_FALSE)
358355
{
359-
Ok(Some(vec![
360-
Interval::CERTAINLY_FALSE,
361-
Interval::CERTAINLY_TRUE,
362-
]))
356+
Ok(Some(vec![Interval::FALSE, Interval::TRUE]))
363357
}
364358
// If both children are uncertain, or if one is certainly false,
365359
// we cannot conclusively refine their intervals. In this case,
@@ -373,33 +367,27 @@ impl PhysicalExpr for BinaryExpr {
373367
Ok(Some(vec![]))
374368
}
375369
} else if self.op.eq(&Operator::Or) {
376-
if interval.eq(&Interval::CERTAINLY_FALSE) {
370+
if interval.eq(&Interval::FALSE) {
377371
// A certainly false logical disjunction can only derive from certainly
378372
// false operands. Otherwise, we prove infeasibility.
379-
Ok((!left_interval.eq(&Interval::CERTAINLY_TRUE)
380-
&& !right_interval.eq(&Interval::CERTAINLY_TRUE))
381-
.then(|| vec![Interval::CERTAINLY_FALSE, Interval::CERTAINLY_FALSE]))
382-
} else if interval.eq(&Interval::CERTAINLY_TRUE) {
373+
Ok((!left_interval.eq(&Interval::TRUE)
374+
&& !right_interval.eq(&Interval::TRUE))
375+
.then(|| vec![Interval::FALSE, Interval::FALSE]))
376+
} else if interval.eq(&Interval::TRUE) {
383377
// If the logical disjunction is certainly true, one of the
384378
// operands must be true. However, it's not always possible to
385379
// determine which operand is true, leading to different scenarios.
386380

387381
// If one operand is certainly false and the other one is uncertain,
388382
// then the latter must be certainly true.
389-
if left_interval.eq(&Interval::CERTAINLY_FALSE)
390-
&& right_interval.eq(&Interval::UNCERTAIN)
383+
if left_interval.eq(&Interval::FALSE)
384+
&& right_interval.eq(&Interval::TRUE_OR_FALSE)
391385
{
392-
Ok(Some(vec![
393-
Interval::CERTAINLY_FALSE,
394-
Interval::CERTAINLY_TRUE,
395-
]))
396-
} else if right_interval.eq(&Interval::CERTAINLY_FALSE)
397-
&& left_interval.eq(&Interval::UNCERTAIN)
386+
Ok(Some(vec![Interval::FALSE, Interval::TRUE]))
387+
} else if right_interval.eq(&Interval::FALSE)
388+
&& left_interval.eq(&Interval::TRUE_OR_FALSE)
398389
{
399-
Ok(Some(vec![
400-
Interval::CERTAINLY_TRUE,
401-
Interval::CERTAINLY_FALSE,
402-
]))
390+
Ok(Some(vec![Interval::TRUE, Interval::FALSE]))
403391
}
404392
// If both children are uncertain, or if one is certainly true,
405393
// we cannot conclusively refine their intervals. In this case,

‎datafusion/physical-expr/src/expressions/not.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,16 +155,16 @@ impl PhysicalExpr for NotExpr {
155155
match (parent, children[0]) {
156156
(Bernoulli(parent), Bernoulli(child)) => {
157157
let parent_range = parent.range();
158-
let result = if parent_range == Interval::CERTAINLY_TRUE {
159-
if child.range() == Interval::CERTAINLY_TRUE {
158+
let result = if parent_range == Interval::TRUE {
159+
if child.range() == Interval::TRUE {
160160
None
161161
} else {
162162
Some(vec![Distribution::new_bernoulli(ScalarValue::new_zero(
163163
&child.data_type(),
164164
)?)?])
165165
}
166-
} else if parent_range == Interval::CERTAINLY_FALSE {
167-
if child.range() == Interval::CERTAINLY_FALSE {
166+
} else if parent_range == Interval::FALSE {
167+
if child.range() == Interval::FALSE {
168168
None
169169
} else {
170170
Some(vec![Distribution::new_bernoulli(ScalarValue::new_one(

0 commit comments

Comments
 (0)