You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by al...@apache.org on 2022/10/10 21:19:53 UTC

[arrow-datafusion] branch master updated: Teach a negative NULL expression to return NULL instead of an error (#3771)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new c02e9d4c7 Teach a negative NULL expression to return NULL instead of an error (#3771)
c02e9d4c7 is described below

commit c02e9d4c7c0aa6716ce9e4a7afd8e882e9efff79
Author: Roman Nozdrin <dr...@gmail.com>
AuthorDate: Tue Oct 11 00:19:48 2022 +0300

    Teach a negative NULL expression to return NULL instead of an error (#3771)
---
 datafusion/core/tests/sql/expr.rs                    | 18 ++++++++++++++++++
 datafusion/expr/src/type_coercion.rs                 |  5 +++++
 datafusion/physical-expr/src/expressions/negative.rs |  9 +++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/datafusion/core/tests/sql/expr.rs b/datafusion/core/tests/sql/expr.rs
index 12344e470..bb9f69dff 100644
--- a/datafusion/core/tests/sql/expr.rs
+++ b/datafusion/core/tests/sql/expr.rs
@@ -646,6 +646,24 @@ async fn test_not_expressions() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn test_negative_expressions() -> Result<()> {
+    let ctx = SessionContext::new();
+
+    let sql = "SELECT null, -null";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+------+----------+",
+        "| NULL | (- NULL) |",
+        "+------+----------+",
+        "|      |          |",
+        "+------+----------+",
+    ];
+
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
 #[tokio::test]
 async fn test_boolean_expressions() -> Result<()> {
     test_expression!("true", "true");
diff --git a/datafusion/expr/src/type_coercion.rs b/datafusion/expr/src/type_coercion.rs
index 41eeb3c65..4a006ad87 100644
--- a/datafusion/expr/src/type_coercion.rs
+++ b/datafusion/expr/src/type_coercion.rs
@@ -48,6 +48,11 @@ pub fn is_signed_numeric(dt: &DataType) -> bool {
     )
 }
 
+// Determine if a DataType is Null or not
+pub fn is_null(dt: &DataType) -> bool {
+    *dt == DataType::Null
+}
+
 /// Determine if a DataType is numeric or not
 pub fn is_numeric(dt: &DataType) -> bool {
     is_signed_numeric(dt)
diff --git a/datafusion/physical-expr/src/expressions/negative.rs b/datafusion/physical-expr/src/expressions/negative.rs
index 233ad4254..4f4def99e 100644
--- a/datafusion/physical-expr/src/expressions/negative.rs
+++ b/datafusion/physical-expr/src/expressions/negative.rs
@@ -30,7 +30,10 @@ use arrow::{
 
 use crate::PhysicalExpr;
 use datafusion_common::{DataFusionError, Result};
-use datafusion_expr::{type_coercion::is_signed_numeric, ColumnarValue};
+use datafusion_expr::{
+    type_coercion::{is_null, is_signed_numeric},
+    ColumnarValue,
+};
 
 /// Invoke a compute kernel on array(s)
 macro_rules! compute_op {
@@ -119,7 +122,9 @@ pub fn negative(
     input_schema: &Schema,
 ) -> Result<Arc<dyn PhysicalExpr>> {
     let data_type = arg.data_type(input_schema)?;
-    if !is_signed_numeric(&data_type) {
+    if is_null(&data_type) {
+        Ok(arg)
+    } else if !is_signed_numeric(&data_type) {
         Err(DataFusionError::Internal(
             format!("Can't create negative physical expr for (- '{:?}'), the type of child expr is {}, not signed numeric", arg, data_type),
         ))