You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ja...@apache.org on 2023/06/29 04:19:47 UTC

[arrow-datafusion] branch main updated: fix: incorrect nullability of between expr (#6786)

This is an automated email from the ASF dual-hosted git repository.

jakevin pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git


The following commit(s) were added to refs/heads/main by this push:
     new 06e22a5386 fix: incorrect nullability of between expr (#6786)
06e22a5386 is described below

commit 06e22a5386a65c0e6a29943cd03ab5e442732745
Author: Jonah Gao <jo...@gmail.com>
AuthorDate: Thu Jun 29 12:19:42 2023 +0800

    fix: incorrect nullability of between expr (#6786)
---
 datafusion/expr/src/expr_schema.rs | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/datafusion/expr/src/expr_schema.rs b/datafusion/expr/src/expr_schema.rs
index 52ad65773c..5c57a8acef 100644
--- a/datafusion/expr/src/expr_schema.rs
+++ b/datafusion/expr/src/expr_schema.rs
@@ -175,7 +175,13 @@ impl ExprSchemable for Expr {
             | Expr::Negative(expr)
             | Expr::Sort(Sort { expr, .. })
             | Expr::InList(InList { expr, .. }) => expr.nullable(input_schema),
-            Expr::Between(Between { expr, .. }) => expr.nullable(input_schema),
+
+            Expr::Between(Between {
+                expr, low, high, ..
+            }) => Ok(expr.nullable(input_schema)?
+                || low.nullable(input_schema)?
+                || high.nullable(input_schema)?),
+
             Expr::Column(c) => input_schema.nullable(c),
             Expr::OuterReferenceColumn(_, _) => Ok(true),
             Expr::Literal(value) => Ok(value.is_null()),
@@ -360,6 +366,30 @@ mod tests {
         test_is_expr_nullable!(is_not_unknown);
     }
 
+    #[test]
+    fn test_between_nullability() {
+        let get_schema = |nullable| {
+            MockExprSchema::new()
+                .with_data_type(DataType::Int32)
+                .with_nullable(nullable)
+        };
+
+        let expr = col("foo").between(lit(1), lit(2));
+        assert!(!expr.nullable(&get_schema(false)).unwrap());
+        assert!(expr.nullable(&get_schema(true)).unwrap());
+
+        let null = lit(ScalarValue::Int32(None));
+
+        let expr = col("foo").between(null.clone(), lit(2));
+        assert!(expr.nullable(&get_schema(false)).unwrap());
+
+        let expr = col("foo").between(lit(1), null.clone());
+        assert!(expr.nullable(&get_schema(false)).unwrap());
+
+        let expr = col("foo").between(null.clone(), null);
+        assert!(expr.nullable(&get_schema(false)).unwrap());
+    }
+
     #[test]
     fn expr_schema_data_type() {
         let expr = col("foo");