fixup tests

Author: davidhewitt

datafusion-examples/examples/expr_api.rs

@@ -424,7 +424,7 @@ fn boundary_analysis_in_conjuctions_demo() -> Result<()> {
     //
     // But `AND` conjunctions are easier to reason with because their interval
     // arithmetic follows naturally from set intersection operations, let us
-    // now look at an example that is a tad more complicated `OR` conjunctions.
+    // now look at an example that is a tad more complicated `OR` disjunctions.
 
     // The expression we will look at is `age > 60 OR age <= 18`.
     let age_greater_than_60_less_than_18 =
@@ -435,7 +435,7 @@ fn boundary_analysis_in_conjuctions_demo() -> Result<()> {
     //
     // Initial range: [14, 79] as described in our column statistics.
     //
-    // From the left-hand side and right-hand side of our `OR` conjunctions
+    // From the left-hand side and right-hand side of our `OR` disjunctions
     // we end up with two ranges, instead of just one.
     //
     // - age > 60: [61, 79]
@@ -446,7 +446,8 @@ fn boundary_analysis_in_conjuctions_demo() -> Result<()> {
     let physical_expr = SessionContext::new()
         .create_physical_expr(age_greater_than_60_less_than_18, &df_schema)?;
 
-    // Since we don't handle interval arithmetic for `OR` operator this will error out.
+    // However, analysis only supports a single interval, so we don't yet deal
+    // with the multiple possibilities of the `OR` disjunctions.
     let analysis = analyze(
         &physical_expr,
         AnalysisContext::new(initial_boundaries),

datafusion/physical-expr/src/analysis.rs

@@ -100,7 +100,7 @@ impl ExprBoundaries {
     ) -> Result<Self> {
         let field = schema.fields().get(col_index).ok_or_else(|| {
             internal_datafusion_err!(
-                "Could not create `ExprBoundaries`: in `try_from_column` `col_index` 
+                "Could not create `ExprBoundaries`: in `try_from_column` `col_index`
                 has gone out of bounds with a value of {col_index}, the schema has {} columns.",
                 schema.fields.len()
             )
@@ -425,7 +425,7 @@ mod tests {
     fn test_analyze_invalid_boundary_exprs() {
         let schema = Arc::new(Schema::new(vec![make_field("a", DataType::Int32)]));
         let expr = col("a").lt(lit(10)).or(col("a").gt(lit(20)));
-        let expected_error = "Interval arithmetic does not support the operator OR";
+        let expected_error = "OR operator cannot yet propagate true intervals";
         let boundaries = ExprBoundaries::try_new_unbounded(&schema).unwrap();
         let df_schema = DFSchema::try_from(Arc::clone(&schema)).unwrap();
         let physical_expr =

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

@@ -516,7 +516,7 @@ impl PhysicalExpr for BinaryExpr {
             }
         } else if self.op.eq(&Operator::Or) {
             if interval.eq(&Interval::CERTAINLY_FALSE) {
-                // A certainly false logical conjunction can only derive from certainly
+                // A certainly false logical disjunction can only derive from certainly
                 // false operands. Otherwise, we prove infeasibility.
                 Ok((!left_interval.eq(&Interval::CERTAINLY_TRUE)
                     && !right_interval.eq(&Interval::CERTAINLY_TRUE))

datafusion/physical-expr/src/intervals/cp_solver.rs

@@ -148,12 +148,12 @@ use std::sync::Arc;
 use super::utils::{
     convert_duration_type_to_interval, convert_interval_type_to_duration, get_inverse_op,
 };
-use crate::expressions::Literal;
+use crate::expressions::{BinaryExpr, Literal};
 use crate::utils::{build_dag, ExprTreeNode};
 use crate::PhysicalExpr;
 
 use arrow::datatypes::{DataType, Schema};
-use datafusion_common::{internal_err, Result};
+use datafusion_common::{internal_err, not_impl_err, Result};
 use datafusion_expr::interval_arithmetic::{apply_operator, satisfy_greater, Interval};
 use datafusion_expr::Operator;
 
@@ -645,6 +645,17 @@ impl ExprIntervalGraph {
                 .map(|child| self.graph[*child].interval())
                 .collect::<Vec<_>>();
             let node_interval = self.graph[node].interval();
+            // Special case: true OR could in principle be propagated by 3 interval sets,
+            // (i.e. left true, or right true, or both true) however we do not support this yet.
+            if node_interval == &Interval::CERTAINLY_TRUE
+                && self.graph[node]
+                    .expr
+                    .as_any()
+                    .downcast_ref::<BinaryExpr>()
+                    .is_some_and(|expr| expr.op() == &Operator::Or)
+            {
+                return not_impl_err!("OR operator cannot yet propagate true intervals");
+            }
             let propagated_intervals = self.graph[node]
                 .expr
                 .propagate_constraints(node_interval, &children_intervals)?;