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 2023/02/13 12:01:55 UTC

[arrow-datafusion] branch master updated: enhance the checking of type errors in the test `window_frame_creation` (#5257)

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 3da790214 enhance the checking of type errors in the test `window_frame_creation` (#5257)
3da790214 is described below

commit 3da790214ea479626eb4114c53440dc17b737d54
Author: Remzi Yang <59...@users.noreply.github.com>
AuthorDate: Mon Feb 13 20:01:50 2023 +0800

    enhance the checking of type errors in the test `window_frame_creation` (#5257)
    
    * enhance the test
    
    Signed-off-by: remzi <13...@gmail.com>
    
    * update comment
    
    Signed-off-by: remzi <13...@gmail.com>
    
    ---------
    
    Signed-off-by: remzi <13...@gmail.com>
---
 datafusion/core/tests/sql/window.rs | 54 ++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/datafusion/core/tests/sql/window.rs b/datafusion/core/tests/sql/window.rs
index 0d39b9433..af03d922c 100644
--- a/datafusion/core/tests/sql/window.rs
+++ b/datafusion/core/tests/sql/window.rs
@@ -1464,22 +1464,50 @@ async fn window_frame_creation() -> Result<()> {
         "Execution error: Invalid window frame: start bound (2 FOLLOWING) cannot be larger than end bound (1 FOLLOWING)"
     );
 
-    let df = ctx
-        .sql(
-            "SELECT
-                COUNT(c1) OVER (ORDER BY c2 RANGE BETWEEN '1 DAY' PRECEDING AND '2 DAY' FOLLOWING)
-                FROM aggregate_test_100;",
-        )
-        .await?;
-    let results = df.collect().await;
-    assert_contains!(
-        results.err().unwrap().to_string(),
-        "Internal error: Operator - is not implemented for types UInt32(1) and Utf8(\"1 DAY\"). This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker"
-    );
-
     Ok(())
 }
 
+#[tokio::test]
+async fn window_frame_creation_type_checking() -> Result<()> {
+    // The following query has type error. We should test the error could be detected
+    // from either the logical plan (when `skip_failed_rules` is set to `false`) or
+    // the physical plan (when `skip_failed_rules` is set to `true`).
+
+    // We should remove the type checking in physical plan after we don't skip
+    // the failed optimizing rules by default. (see more in https://github.com/apache/arrow-datafusion/issues/4615)
+    async fn check_query(skip_failed_rules: bool, err_msg: &str) -> Result<()> {
+        use datafusion_common::ScalarValue::Boolean;
+        let config = SessionConfig::new().set(
+            "datafusion.optimizer.skip_failed_rules",
+            Boolean(Some(skip_failed_rules)),
+        );
+        let ctx = SessionContext::with_config(config);
+        register_aggregate_csv(&ctx).await?;
+        let df = ctx
+            .sql(
+                "SELECT
+                    COUNT(c1) OVER (ORDER BY c2 RANGE BETWEEN '1 DAY' PRECEDING AND '2 DAY' FOLLOWING)
+                    FROM aggregate_test_100;",
+            )
+            .await?;
+        let results = df.collect().await;
+        assert_contains!(results.err().unwrap().to_string(), err_msg);
+        Ok(())
+    }
+
+    // Error is returned from the physical plan.
+    check_query(
+        true,
+        "Internal error: Operator - is not implemented for types UInt32(1) and Utf8(\"1 DAY\")."
+    ).await?;
+
+    // Error is returned from the logical plan.
+    check_query(
+        false,
+        "Internal error: Optimizer rule 'type_coercion' failed due to unexpected error: Arrow error: Cast error: Cannot cast string '1 DAY' to value of UInt32 type"
+    ).await
+}
+
 #[tokio::test]
 async fn test_window_row_number_aggregate() -> Result<()> {
     let config = SessionConfig::new();