You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/02 10:59:30 UTC

[GitHub] [arrow-datafusion] alamb commented on a change in pull request #1969: Coalesce function

alamb commented on a change in pull request #1969:
URL: https://github.com/apache/arrow-datafusion/pull/1969#discussion_r841061741



##########
File path: datafusion/core/tests/sql/functions.rs
##########
@@ -174,3 +174,202 @@ async fn query_count_distinct() -> Result<()> {
     assert_batches_eq!(expected, &actual);
     Ok(())
 }
+
+#[tokio::test]
+async fn coalesce_plan() -> Result<()> {
+    let ctx = SessionContext::new();
+    let sql = "select COALESCE('', 'test')";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------------+",
+        "| coalesce(Utf8(\"\"),Utf8(\"test\")) |",
+        "+---------------------------------+",
+        "|                                 |",
+        "+---------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_result() -> Result<()> {
+    let schema = Arc::new(Schema::new(vec![
+        Field::new("c1", DataType::Int32, true),
+        Field::new("c2", DataType::Int32, true),
+    ]));
+
+    let data = RecordBatch::try_new(
+        schema.clone(),
+        vec![
+            Arc::new(Int32Array::from(vec![Some(0), None, Some(1), None, None])),
+            Arc::new(Int32Array::from(vec![
+                Some(1),
+                Some(1),
+                Some(0),
+                Some(1),
+                None,
+            ])),
+        ],
+    )?;
+
+    let table = MemTable::try_new(schema, vec![vec![data]])?;
+
+    let ctx = SessionContext::new();
+    ctx.register_table("test", Arc::new(table))?;
+    let sql = "SELECT COALESCE(c1, c2) FROM test";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------+",
+        "| coalesce(test.c1,test.c2) |",
+        "+---------------------------+",
+        "| 0                         |",
+        "| 1                         |",

Review comment:
       👍 

##########
File path: datafusion/core/tests/sql/functions.rs
##########
@@ -174,3 +174,202 @@ async fn query_count_distinct() -> Result<()> {
     assert_batches_eq!(expected, &actual);
     Ok(())
 }
+
+#[tokio::test]
+async fn coalesce_plan() -> Result<()> {
+    let ctx = SessionContext::new();
+    let sql = "select COALESCE('', 'test')";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------------+",
+        "| coalesce(Utf8(\"\"),Utf8(\"test\")) |",
+        "+---------------------------------+",
+        "|                                 |",
+        "+---------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_result() -> Result<()> {
+    let schema = Arc::new(Schema::new(vec![
+        Field::new("c1", DataType::Int32, true),
+        Field::new("c2", DataType::Int32, true),
+    ]));
+
+    let data = RecordBatch::try_new(
+        schema.clone(),
+        vec![
+            Arc::new(Int32Array::from(vec![Some(0), None, Some(1), None, None])),
+            Arc::new(Int32Array::from(vec![
+                Some(1),
+                Some(1),
+                Some(0),
+                Some(1),
+                None,
+            ])),
+        ],
+    )?;
+
+    let table = MemTable::try_new(schema, vec![vec![data]])?;
+
+    let ctx = SessionContext::new();
+    ctx.register_table("test", Arc::new(table))?;
+    let sql = "SELECT COALESCE(c1, c2) FROM test";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------+",
+        "| coalesce(test.c1,test.c2) |",
+        "+---------------------------+",
+        "| 0                         |",
+        "| 1                         |",
+        "| 1                         |",
+        "| 1                         |",
+        "|                           |",
+        "+---------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_static_empty_value() -> Result<()> {
+    let ctx = SessionContext::new();
+    let sql = "SELECT COALESCE('', 'test')";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------------+",
+        "| coalesce(Utf8(\"\"),Utf8(\"test\")) |",
+        "+---------------------------------+",
+        "|                                 |",
+        "+---------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_static_value_with_null() -> Result<()> {
+    let ctx = SessionContext::new();
+    let sql = "SELECT COALESCE(NULL, 'test')";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+-----------------------------------+",
+        "| coalesce(Utf8(NULL),Utf8(\"test\")) |",
+        "+-----------------------------------+",
+        "| test                              |",
+        "+-----------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_result_with_default_value() -> Result<()> {
+    let schema = Arc::new(Schema::new(vec![
+        Field::new("c1", DataType::Int32, true),
+        Field::new("c2", DataType::Int32, true),
+    ]));
+
+    let data = RecordBatch::try_new(
+        schema.clone(),
+        vec![
+            Arc::new(Int32Array::from(vec![Some(0), None, Some(1), None, None])),
+            Arc::new(Int32Array::from(vec![
+                Some(1),
+                Some(1),
+                Some(0),
+                Some(1),
+                None,
+            ])),
+        ],
+    )?;
+
+    let table = MemTable::try_new(schema, vec![vec![data]])?;
+
+    let ctx = SessionContext::new();
+    ctx.register_table("test", Arc::new(table))?;
+    let sql = "SELECT COALESCE(c1, c2, '-1') FROM test";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+--------------------------------------+",
+        "| coalesce(test.c1,test.c2,Utf8(\"-1\")) |",
+        "+--------------------------------------+",
+        "| 0                                    |",
+        "| 1                                    |",
+        "| 1                                    |",
+        "| 1                                    |",
+        "| -1                                   |",
+        "+--------------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_sum_with_default_value() -> Result<()> {
+    let schema = Arc::new(Schema::new(vec![
+        Field::new("c1", DataType::Int32, true),
+        Field::new("c2", DataType::Int32, true),
+    ]));
+
+    let data = RecordBatch::try_new(
+        schema.clone(),
+        vec![
+            Arc::new(Int32Array::from(vec![Some(1), None, Some(1), None])),
+            Arc::new(Int32Array::from(vec![Some(2), Some(2), None, None])),
+        ],
+    )?;
+
+    let table = MemTable::try_new(schema, vec![vec![data]])?;
+
+    let ctx = SessionContext::new();
+    ctx.register_table("test", Arc::new(table))?;
+    let sql = "SELECT SUM(COALESCE(c1, c2, 0)) FROM test";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+-----------------------------------------+",
+        "| SUM(coalesce(test.c1,test.c2,Int64(0))) |",
+        "+-----------------------------------------+",
+        "| 4                                       |",
+        "+-----------------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_mul_with_default_value() -> Result<()> {
+    let schema = Arc::new(Schema::new(vec![
+        Field::new("c1", DataType::Int32, true),
+        Field::new("c2", DataType::Int32, true),
+    ]));
+
+    let data = RecordBatch::try_new(
+        schema.clone(),
+        vec![
+            Arc::new(Int32Array::from(vec![Some(1), None, Some(1), None])),
+            Arc::new(Int32Array::from(vec![Some(2), Some(2), None, None])),
+        ],
+    )?;
+
+    let table = MemTable::try_new(schema, vec![vec![data]])?;
+
+    let ctx = SessionContext::new();
+    ctx.register_table("test", Arc::new(table))?;
+    let sql = "SELECT COALESCE(c1 * c2, 0) FROM test";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------------------------+",
+        "| coalesce(test.c1 Multiply test.c2,Int64(0)) |",
+        "+---------------------------------------------+",
+        "| 2                                           |",
+        "| 0                                           |",
+        "| 0                                           |",
+        "| 0                                           |",
+        "+---------------------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())

Review comment:
       nice testing 👍 

##########
File path: datafusion/expr/src/expr_fn.rs
##########
@@ -297,6 +297,14 @@ pub fn array(args: Vec<Expr>) -> Expr {
     }
 }
 
+/// Returns an coalesce of fixed size with each argument on it.

Review comment:
       ```suggestion
   /// Returns `coalesce(args...)`, which evaluates to the value of the first [Expr] 
   /// which is not NULL
   ```

##########
File path: datafusion/core/tests/sql/functions.rs
##########
@@ -174,3 +174,202 @@ async fn query_count_distinct() -> Result<()> {
     assert_batches_eq!(expected, &actual);
     Ok(())
 }
+
+#[tokio::test]
+async fn coalesce_plan() -> Result<()> {
+    let ctx = SessionContext::new();
+    let sql = "select COALESCE('', 'test')";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------------+",
+        "| coalesce(Utf8(\"\"),Utf8(\"test\")) |",
+        "+---------------------------------+",
+        "|                                 |",
+        "+---------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_result() -> Result<()> {
+    let schema = Arc::new(Schema::new(vec![
+        Field::new("c1", DataType::Int32, true),
+        Field::new("c2", DataType::Int32, true),
+    ]));
+
+    let data = RecordBatch::try_new(
+        schema.clone(),
+        vec![
+            Arc::new(Int32Array::from(vec![Some(0), None, Some(1), None, None])),
+            Arc::new(Int32Array::from(vec![
+                Some(1),
+                Some(1),
+                Some(0),
+                Some(1),
+                None,
+            ])),
+        ],
+    )?;
+
+    let table = MemTable::try_new(schema, vec![vec![data]])?;
+
+    let ctx = SessionContext::new();
+    ctx.register_table("test", Arc::new(table))?;
+    let sql = "SELECT COALESCE(c1, c2) FROM test";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------+",
+        "| coalesce(test.c1,test.c2) |",
+        "+---------------------------+",
+        "| 0                         |",
+        "| 1                         |",
+        "| 1                         |",
+        "| 1                         |",
+        "|                           |",
+        "+---------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_static_empty_value() -> Result<()> {
+    let ctx = SessionContext::new();
+    let sql = "SELECT COALESCE('', 'test')";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------------+",
+        "| coalesce(Utf8(\"\"),Utf8(\"test\")) |",
+        "+---------------------------------+",
+        "|                                 |",
+        "+---------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_static_value_with_null() -> Result<()> {
+    let ctx = SessionContext::new();
+    let sql = "SELECT COALESCE(NULL, 'test')";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+-----------------------------------+",
+        "| coalesce(Utf8(NULL),Utf8(\"test\")) |",
+        "+-----------------------------------+",
+        "| test                              |",
+        "+-----------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_result_with_default_value() -> Result<()> {

Review comment:
       stylistically this might be better if it were right next to `coalesce_result` given its similarity.
   
   For that matter, since it uses the same data setup, it might be cool to combine the two tests (and run two different queries in them)

##########
File path: datafusion/core/tests/sql/functions.rs
##########
@@ -174,3 +174,202 @@ async fn query_count_distinct() -> Result<()> {
     assert_batches_eq!(expected, &actual);
     Ok(())
 }
+
+#[tokio::test]
+async fn coalesce_plan() -> Result<()> {
+    let ctx = SessionContext::new();
+    let sql = "select COALESCE('', 'test')";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------------+",
+        "| coalesce(Utf8(\"\"),Utf8(\"test\")) |",
+        "+---------------------------------+",
+        "|                                 |",
+        "+---------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_result() -> Result<()> {
+    let schema = Arc::new(Schema::new(vec![
+        Field::new("c1", DataType::Int32, true),
+        Field::new("c2", DataType::Int32, true),
+    ]));
+
+    let data = RecordBatch::try_new(
+        schema.clone(),
+        vec![
+            Arc::new(Int32Array::from(vec![Some(0), None, Some(1), None, None])),
+            Arc::new(Int32Array::from(vec![
+                Some(1),
+                Some(1),
+                Some(0),
+                Some(1),
+                None,
+            ])),
+        ],
+    )?;
+
+    let table = MemTable::try_new(schema, vec![vec![data]])?;
+
+    let ctx = SessionContext::new();
+    ctx.register_table("test", Arc::new(table))?;
+    let sql = "SELECT COALESCE(c1, c2) FROM test";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------+",
+        "| coalesce(test.c1,test.c2) |",
+        "+---------------------------+",
+        "| 0                         |",
+        "| 1                         |",
+        "| 1                         |",
+        "| 1                         |",
+        "|                           |",
+        "+---------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_static_empty_value() -> Result<()> {

Review comment:
       this test seems to be the same as `coalesce_plan` -- is that intentional?

##########
File path: datafusion/core/tests/sql/functions.rs
##########
@@ -174,3 +174,202 @@ async fn query_count_distinct() -> Result<()> {
     assert_batches_eq!(expected, &actual);
     Ok(())
 }
+
+#[tokio::test]
+async fn coalesce_plan() -> Result<()> {
+    let ctx = SessionContext::new();
+    let sql = "select COALESCE('', 'test')";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------------+",
+        "| coalesce(Utf8(\"\"),Utf8(\"test\")) |",
+        "+---------------------------------+",
+        "|                                 |",
+        "+---------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_result() -> Result<()> {
+    let schema = Arc::new(Schema::new(vec![
+        Field::new("c1", DataType::Int32, true),
+        Field::new("c2", DataType::Int32, true),
+    ]));
+
+    let data = RecordBatch::try_new(
+        schema.clone(),
+        vec![
+            Arc::new(Int32Array::from(vec![Some(0), None, Some(1), None, None])),
+            Arc::new(Int32Array::from(vec![
+                Some(1),
+                Some(1),
+                Some(0),
+                Some(1),
+                None,
+            ])),
+        ],
+    )?;
+
+    let table = MemTable::try_new(schema, vec![vec![data]])?;
+
+    let ctx = SessionContext::new();
+    ctx.register_table("test", Arc::new(table))?;
+    let sql = "SELECT COALESCE(c1, c2) FROM test";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------+",
+        "| coalesce(test.c1,test.c2) |",
+        "+---------------------------+",
+        "| 0                         |",
+        "| 1                         |",
+        "| 1                         |",
+        "| 1                         |",
+        "|                           |",
+        "+---------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_static_empty_value() -> Result<()> {
+    let ctx = SessionContext::new();
+    let sql = "SELECT COALESCE('', 'test')";
+    let actual = execute_to_batches(&ctx, sql).await;
+    let expected = vec![
+        "+---------------------------------+",
+        "| coalesce(Utf8(\"\"),Utf8(\"test\")) |",
+        "+---------------------------------+",
+        "|                                 |",
+        "+---------------------------------+",
+    ];
+    assert_batches_eq!(expected, &actual);
+    Ok(())
+}
+
+#[tokio::test]
+async fn coalesce_static_value_with_null() -> Result<()> {

Review comment:
       👍 

##########
File path: datafusion/physical-expr/src/conditional_expressions.rs
##########
@@ -0,0 +1,86 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::sync::Arc;
+
+use arrow::array::{new_null_array, Array, BooleanArray};
+use arrow::compute;
+use arrow::compute::kernels::zip::zip;
+use arrow::datatypes::DataType;
+
+use datafusion_common::{DataFusionError, Result};
+use datafusion_expr::ColumnarValue;
+
+pub fn coalesce(args: &[ColumnarValue]) -> Result<ColumnarValue> {

Review comment:
       ```suggestion
   /// coalesce evaluates to the first value which is not NULL
   pub fn coalesce(args: &[ColumnarValue]) -> Result<ColumnarValue> {
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org