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/09/09 14:50:19 UTC

[GitHub] [arrow-datafusion] liukun4515 opened a new pull request, #3422: support cast/try_cast in prune

liukun4515 opened a new pull request, #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422

   # Which issue does this PR close?
   
   <!--
   We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
   -->
   
   Closes https://github.com/apache/arrow-datafusion/issues/3414
   
   In order to merge this  https://github.com/apache/arrow-datafusion/pull/3396, we should support the cast/try_cast in the prune first.
   
   cc @andygrove @alamb 
   
    # Rationale for this change
   <!--
    Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
    Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.  
   -->
   
   # What changes are included in this PR?
   <!--
   There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
   -->
   
   # Are there any user-facing changes?
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!--
   If there are any breaking changes to public APIs, please add the `api change` label.
   -->


-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on pull request #3422: support cast/try_cast in prune

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#issuecomment-1242600435

   > Thank you @liukun4515 -- this is great
   > 
   > I will look at this code in the morning as well, but I think we need to handle the case where casting won't preserve sortedness (aka UTF8 -> numeric), as described on #3377
   
   I got your point.
   
   We can add rule for cast/try_cast case.
   
   For example, just support cast/try_cast from numeric to numeric data type, I think the data type of numeric has the order.
   
   We can support other complex case in the future pr.
   
   @alamb 


-- 
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


[GitHub] [arrow-datafusion] alamb commented on pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
alamb commented on PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#issuecomment-1242709855

   To be explicit I think the code in this PR looks good to me, but I would like to see some actual tests with pruning expressions on data prior to merging it. I am happy to help write these tests too


-- 
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


[GitHub] [arrow-datafusion] Dandandan commented on a diff in pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
Dandandan commented on code in PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#discussion_r968035105


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -495,22 +500,30 @@ fn rewrite_expr_to_prunable(
     }
 
     match column_expr {
-        // `col > lit()`
+        // col op lit()
         Expr::Column(_) => Ok((column_expr.clone(), op, scalar_expr.clone())),
-
+        // `cast(col) op lit()`
+        Expr::Cast { expr, data_type } => {
+            let from_type = expr.get_type(&schema)?;
+            verify_support_type_for_prune(&from_type, data_type)?;
+            let (left, op, right) =
+                rewrite_expr_to_prunable(expr, op, scalar_expr, schema)?;
+            Ok((cast(left, data_type.clone()), op, right))
+        }
+        // `try_cast(col) op lit()`
+        Expr::TryCast { expr, data_type } => {
+            let from_type = expr.get_type(&schema)?;
+            verify_support_type_for_prune(&from_type, data_type)?;
+            let (left, op, right) =
+                rewrite_expr_to_prunable(expr, op, scalar_expr, schema)?;
+            Ok((try_cast(left, data_type.clone()), op, right))
+        }
+        // `col > lit()`

Review Comment:
   This comment should be removed I think?



-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#issuecomment-1242747814

   > To be explicit I think the code in this PR looks good to me, but I would like to see some actual tests with pruning expressions on data prior to merging it. I am happy to help write these tests too
   
   @alamb  Thanks for you help.
   It's too later for me to add test today, but welcome to add test for this pr, I can help to review your test and other changes tomorrow.
   I will never do any changes before merged your test or your suggestion.
   
   


-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on a diff in pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on code in PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#discussion_r967723715


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -552,6 +577,25 @@ fn is_compare_op(op: Operator) -> bool {
     )
 }
 
+// The pruning logic is based on the comparing the min/max bounds.
+// Must make sure the two type has order.
+// For example, casts from string to numbers is not correct.
+// Because the "13" is less than "3" with UTF8 comparison order.
+fn support_type_for_prune(from_type: &DataType, to_type: &DataType) -> bool {
+    // TODO: support other data type

Review Comment:
   https://github.com/apache/arrow-datafusion/issues/3442



-- 
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


[GitHub] [arrow-datafusion] alamb merged pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
alamb merged PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422


-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on a diff in pull request #3422: support cast/try_cast in prune

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on code in PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#discussion_r967617783


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -1508,6 +1510,78 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn row_group_predicate_cast() -> Result<()> {
+        let schema = Schema::new(vec![Field::new("c1", DataType::Int32, false)]);
+        let expected_expr =
+            "CAST(#c1_min AS Int64) <= Int64(1) AND Int64(1) <= CAST(#c1_max AS Int64)";
+
+        // test column on the left
+        let expr = cast(col("c1"), DataType::Int64).eq(lit(ScalarValue::Int64(Some(1))));
+        let predicate_expr =
+            build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
+        assert_eq!(format!("{:?}", predicate_expr), expected_expr);
+
+        // test column on the right
+        let expr = lit(ScalarValue::Int64(Some(1))).eq(cast(col("c1"), DataType::Int64));
+        let predicate_expr =
+            build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
+        assert_eq!(format!("{:?}", predicate_expr), expected_expr);
+
+        let expected_expr = "TRY_CAST(#c1_max AS Int64) > Int64(1)";
+
+        // test column on the left
+        let expr =
+            try_cast(col("c1"), DataType::Int64).gt(lit(ScalarValue::Int64(Some(1))));
+        let predicate_expr =
+            build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
+        assert_eq!(format!("{:?}", predicate_expr), expected_expr);
+
+        // test column on the right
+        let expr =
+            lit(ScalarValue::Int64(Some(1))).lt(try_cast(col("c1"), DataType::Int64));
+        let predicate_expr =
+            build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
+        assert_eq!(format!("{:?}", predicate_expr), expected_expr);
+
+        Ok(())
+    }
+
+    #[test]
+    fn row_group_predicate_cast_list() -> Result<()> {
+        let schema = Schema::new(vec![Field::new("c1", DataType::Int32, false)]);
+        // test cast(c1 as int64) in int64(1, 2, 3)
+        let expr = Expr::InList {
+            expr: Box::new(cast(col("c1"), DataType::Int64)),
+            list: vec![
+                lit(ScalarValue::Int64(Some(1))),
+                lit(ScalarValue::Int64(Some(2))),
+                lit(ScalarValue::Int64(Some(3))),
+            ],
+            negated: false,
+        };
+        let expected_expr = "CAST(#c1_min AS Int64) <= Int64(1) AND Int64(1) <= CAST(#c1_max AS Int64) OR CAST(#c1_min AS Int64) <= Int64(2) AND Int64(2) <= CAST(#c1_max AS Int64) OR CAST(#c1_min AS Int64) <= Int64(3) AND Int64(3) <= CAST(#c1_max AS Int64)";
+        let predicate_expr =
+            build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
+        assert_eq!(format!("{:?}", predicate_expr), expected_expr);
+
+        let expr = Expr::InList {
+            expr: Box::new(cast(col("c1"), DataType::Int64)),
+            list: vec![
+                lit(ScalarValue::Int64(Some(1))),
+                lit(ScalarValue::Int64(Some(2))),
+                lit(ScalarValue::Int64(Some(3))),
+            ],
+            negated: true,
+        };
+        let expected_expr = "CAST(#c1_min AS Int64) != Int64(1) OR Int64(1) != CAST(#c1_max AS Int64) AND CAST(#c1_min AS Int64) != Int64(2) OR Int64(2) != CAST(#c1_max AS Int64) AND CAST(#c1_min AS Int64) != Int64(3) OR Int64(3) != CAST(#c1_max AS Int64)";

Review Comment:
   @andygrove I don't change any logical about the list.
   you can get this from https://github.com/apache/arrow-datafusion/pull/2282#discussion_r855486677
   
   I get the meanings from the code context.
   if the min_c1 and max_c1 is 1, all the value in the column or chunk must be 1 or null, so the `c1 not in (1)` can skip this column or chunk.



-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on a diff in pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on code in PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#discussion_r967722831


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -495,21 +501,40 @@ fn rewrite_expr_to_prunable(
     }
 
     match column_expr {
+        // col op lit()
+        Expr::Column(_) => Ok((column_expr.clone(), op, scalar_expr.clone())),
         // `cast(col) op lit()`
         Expr::Cast { expr, data_type } => {
-            let (left, op, right) = rewrite_expr_to_prunable(expr, op, scalar_expr)?;
-            Ok((cast(left, data_type.clone()), op, right))
+            let from_type = expr.get_type(&schema)?;
+            if support_type_for_prune(&from_type, data_type) {
+                let (left, op, right) =
+                    rewrite_expr_to_prunable(expr, op, scalar_expr, schema)?;
+                Ok((cast(left, data_type.clone()), op, right))
+            } else {
+                return Err(DataFusionError::Plan(format!(
+                    "Cast with from type {} to type {} is not supported",
+                    from_type, data_type
+                )));
+            }

Review Comment:
   good point



-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#issuecomment-1242686512

   cc @alamb 
   The cast/try_cast just support signed integer/decimal to signed integer/decimal.


-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on a diff in pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on code in PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#discussion_r968329166


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -495,22 +500,30 @@ fn rewrite_expr_to_prunable(
     }
 
     match column_expr {
-        // `col > lit()`
+        // col op lit()
         Expr::Column(_) => Ok((column_expr.clone(), op, scalar_expr.clone())),
-
+        // `cast(col) op lit()`
+        Expr::Cast { expr, data_type } => {
+            let from_type = expr.get_type(&schema)?;
+            verify_support_type_for_prune(&from_type, data_type)?;
+            let (left, op, right) =
+                rewrite_expr_to_prunable(expr, op, scalar_expr, schema)?;
+            Ok((cast(left, data_type.clone()), op, right))
+        }
+        // `try_cast(col) op lit()`
+        Expr::TryCast { expr, data_type } => {
+            let from_type = expr.get_type(&schema)?;
+            verify_support_type_for_prune(&from_type, data_type)?;
+            let (left, op, right) =
+                rewrite_expr_to_prunable(expr, op, scalar_expr, schema)?;
+            Ok((try_cast(left, data_type.clone()), op, right))
+        }
+        // `col > lit()`

Review Comment:
   Done
   removed



-- 
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


[GitHub] [arrow-datafusion] ursabot commented on pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#issuecomment-1244085625

   Benchmark runs are scheduled for baseline = 3b8a20a94eb12ec393072694e7e278e32223fcd6 and contender = 69d05aa0c563a478e28502c0a4e7822095859b28. 69d05aa0c563a478e28502c0a4e7822095859b28 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/75e3f5a234134c4faf7cc5f4729cba24...d3378d8e9f614d2792e91881e045b411/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/d654ca62429040409c0a13bfdc2b11ad...54fe81f8bfa941e18efe03f0127d7c11/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/c54755ec9bbb47a99c4fbec4a4c22039...0c3b235735c7479095f2223a906e8ce3/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/5fa141748a2f4f46b1d8a2b4563d9678...ebdf8e35153641d78e06e2c2b3ad0da5/)
   Buildkite builds:
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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


[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3422: support cast/try_cast in prune

Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#discussion_r967425770


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -1508,6 +1510,78 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn row_group_predicate_cast() -> Result<()> {
+        let schema = Schema::new(vec![Field::new("c1", DataType::Int32, false)]);
+        let expected_expr =
+            "CAST(#c1_min AS Int64) <= Int64(1) AND Int64(1) <= CAST(#c1_max AS Int64)";
+
+        // test column on the left
+        let expr = cast(col("c1"), DataType::Int64).eq(lit(ScalarValue::Int64(Some(1))));
+        let predicate_expr =
+            build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
+        assert_eq!(format!("{:?}", predicate_expr), expected_expr);
+
+        // test column on the right
+        let expr = lit(ScalarValue::Int64(Some(1))).eq(cast(col("c1"), DataType::Int64));
+        let predicate_expr =
+            build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
+        assert_eq!(format!("{:?}", predicate_expr), expected_expr);
+
+        let expected_expr = "TRY_CAST(#c1_max AS Int64) > Int64(1)";
+
+        // test column on the left
+        let expr =
+            try_cast(col("c1"), DataType::Int64).gt(lit(ScalarValue::Int64(Some(1))));
+        let predicate_expr =
+            build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
+        assert_eq!(format!("{:?}", predicate_expr), expected_expr);
+
+        // test column on the right
+        let expr =
+            lit(ScalarValue::Int64(Some(1))).lt(try_cast(col("c1"), DataType::Int64));
+        let predicate_expr =
+            build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
+        assert_eq!(format!("{:?}", predicate_expr), expected_expr);
+
+        Ok(())
+    }
+
+    #[test]
+    fn row_group_predicate_cast_list() -> Result<()> {
+        let schema = Schema::new(vec![Field::new("c1", DataType::Int32, false)]);
+        // test cast(c1 as int64) in int64(1, 2, 3)
+        let expr = Expr::InList {
+            expr: Box::new(cast(col("c1"), DataType::Int64)),
+            list: vec![
+                lit(ScalarValue::Int64(Some(1))),
+                lit(ScalarValue::Int64(Some(2))),
+                lit(ScalarValue::Int64(Some(3))),
+            ],
+            negated: false,
+        };
+        let expected_expr = "CAST(#c1_min AS Int64) <= Int64(1) AND Int64(1) <= CAST(#c1_max AS Int64) OR CAST(#c1_min AS Int64) <= Int64(2) AND Int64(2) <= CAST(#c1_max AS Int64) OR CAST(#c1_min AS Int64) <= Int64(3) AND Int64(3) <= CAST(#c1_max AS Int64)";
+        let predicate_expr =
+            build_predicate_expression(&expr, &schema, &mut RequiredStatColumns::new())?;
+        assert_eq!(format!("{:?}", predicate_expr), expected_expr);
+
+        let expr = Expr::InList {
+            expr: Box::new(cast(col("c1"), DataType::Int64)),
+            list: vec![
+                lit(ScalarValue::Int64(Some(1))),
+                lit(ScalarValue::Int64(Some(2))),
+                lit(ScalarValue::Int64(Some(3))),
+            ],
+            negated: true,
+        };
+        let expected_expr = "CAST(#c1_min AS Int64) != Int64(1) OR Int64(1) != CAST(#c1_max AS Int64) AND CAST(#c1_min AS Int64) != Int64(2) OR Int64(2) != CAST(#c1_max AS Int64) AND CAST(#c1_min AS Int64) != Int64(3) OR Int64(3) != CAST(#c1_max AS Int64)";

Review Comment:
   I'm not sure I understand what is happening with the negated case. Could you add a comment here explaining this? Why is there `!=` in here rather than `<=` or `>=`?



-- 
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


[GitHub] [arrow-datafusion] codecov-commenter commented on pull request #3422: support cast/try_cast in prune

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#issuecomment-1242140100

   # [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/3422?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3422](https://codecov.io/gh/apache/arrow-datafusion/pull/3422?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2d77a8d) into [master](https://codecov.io/gh/apache/arrow-datafusion/commit/e6378f40ebc004ff63128eb6c0f59b6242479ea7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e6378f4) will **increase** coverage by `0.07%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #3422      +/-   ##
   ==========================================
   + Coverage   85.58%   85.66%   +0.07%     
   ==========================================
     Files         296      296              
     Lines       54252    54544     +292     
   ==========================================
   + Hits        46432    46723     +291     
   - Misses       7820     7821       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-datafusion/pull/3422?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ” | |
   |---|---|---|
   | [datafusion/core/src/physical\_optimizer/pruning.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9waHlzaWNhbF9vcHRpbWl6ZXIvcHJ1bmluZy5ycw==) | `95.54% <100.00%> (+0.79%)` | :arrow_up: |
   | [datafusion/expr/src/expr\_fn.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy9leHByX2ZuLnJz) | `91.16% <100.00%> (+0.09%)` | :arrow_up: |
   | [benchmarks/src/bin/tpch.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmVuY2htYXJrcy9zcmMvYmluL3RwY2gucnM=) | `37.59% <0.00%> (-3.56%)` | :arrow_down: |
   | [datafusion/physical-expr/src/planner.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9wbGFubmVyLnJz) | `92.68% <0.00%> (-1.52%)` | :arrow_down: |
   | [datafusion/optimizer/src/optimizer.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9vcHRpbWl6ZXIvc3JjL29wdGltaXplci5ycw==) | `90.90% <0.00%> (-1.40%)` | :arrow_down: |
   | [datafusion/expr/src/window\_frame.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy93aW5kb3dfZnJhbWUucnM=) | `92.43% <0.00%> (-0.85%)` | :arrow_down: |
   | [datafusion/sql/src/planner.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9zcWwvc3JjL3BsYW5uZXIucnM=) | `81.06% <0.00%> (-0.62%)` | :arrow_down: |
   | [datafusion/optimizer/src/reduce\_outer\_join.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9vcHRpbWl6ZXIvc3JjL3JlZHVjZV9vdXRlcl9qb2luLnJz) | `98.19% <0.00%> (-0.61%)` | :arrow_down: |
   | [datafusion/core/src/physical\_plan/planner.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9waHlzaWNhbF9wbGFuL3BsYW5uZXIucnM=) | `76.87% <0.00%> (-0.58%)` | :arrow_down: |
   | [datafusion/proto/src/logical\_plan.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9wcm90by9zcmMvbG9naWNhbF9wbGFuLnJz) | `17.46% <0.00%> (-0.24%)` | :arrow_down: |
   | ... and [20 more](https://codecov.io/gh/apache/arrow-datafusion/pull/3422/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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


[GitHub] [arrow-datafusion] alamb commented on pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
alamb commented on PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#issuecomment-1244028826

   Follow on PR is https://github.com/apache/arrow-datafusion/pull/3454


-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on a diff in pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on code in PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#discussion_r967720763


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -1995,4 +2039,66 @@ mod tests {
         let result = p.prune(&statistics).unwrap();
         assert_eq!(result, expected_ret);
     }
+
+    #[test]
+    fn test_rewrite_expr_to_prunable() {
+        let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
+        let df_schema = DFSchema::try_from(schema).unwrap();
+        // column op lit
+        let left_input = col("a");
+        let right_input = lit(ScalarValue::Int32(Some(12)));
+        let (result_left, _, result_right) = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Eq,
+            &right_input,
+            df_schema.clone(),
+        )
+        .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // cast op lit
+        let left_input = cast(col("a"), DataType::Decimal128(20, 3));
+        let right_input = lit(ScalarValue::Decimal128(Some(12), 20, 3));
+        let (result_left, _, result_right) = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Gt,
+            &right_input,
+            df_schema.clone(),
+        )
+        .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // try_cast op lit
+        let left_input = try_cast(col("a"), DataType::Int64);
+        let right_input = lit(ScalarValue::Int64(Some(12)));
+        let (result_left, _, result_right) =
+            rewrite_expr_to_prunable(&left_input, Operator::Gt, &right_input, df_schema)
+                .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // TODO: add test for other case and op

Review Comment:
   I find i have added the test for executing with real data in the `prune_cast_column_scalar` function.
   
   You can check them, it just include cast(col, int64) and try_cast(col,int64). If you think it is not enough, you can add more test for it or give some advices for the test.



-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on a diff in pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on code in PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#discussion_r967720852


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -1995,4 +2039,66 @@ mod tests {
         let result = p.prune(&statistics).unwrap();
         assert_eq!(result, expected_ret);
     }
+
+    #[test]
+    fn test_rewrite_expr_to_prunable() {
+        let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
+        let df_schema = DFSchema::try_from(schema).unwrap();
+        // column op lit
+        let left_input = col("a");
+        let right_input = lit(ScalarValue::Int32(Some(12)));
+        let (result_left, _, result_right) = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Eq,
+            &right_input,
+            df_schema.clone(),
+        )
+        .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // cast op lit
+        let left_input = cast(col("a"), DataType::Decimal128(20, 3));
+        let right_input = lit(ScalarValue::Decimal128(Some(12), 20, 3));
+        let (result_left, _, result_right) = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Gt,
+            &right_input,
+            df_schema.clone(),
+        )
+        .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // try_cast op lit
+        let left_input = try_cast(col("a"), DataType::Int64);
+        let right_input = lit(ScalarValue::Int64(Some(12)));
+        let (result_left, _, result_right) =
+            rewrite_expr_to_prunable(&left_input, Operator::Gt, &right_input, df_schema)
+                .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // TODO: add test for other case and op

Review Comment:
   https://github.com/apache/arrow-datafusion/blob/4b1a4052d079c8149ad232a05e9965cceb05b5fc/datafusion/core/src/physical_optimizer/pruning.rs#L2011
   
   https://github.com/apache/arrow-datafusion/blob/4b1a4052d079c8149ad232a05e9965cceb05b5fc/datafusion/core/src/physical_optimizer/pruning.rs#L2022
   
   https://github.com/apache/arrow-datafusion/blob/4b1a4052d079c8149ad232a05e9965cceb05b5fc/datafusion/core/src/physical_optimizer/pruning.rs#L2028
   
   FYI @alamb 



-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on a diff in pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on code in PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#discussion_r967721530


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -429,11 +429,16 @@ impl<'a> PruningExpressionBuilder<'a> {
                 }
             };
 
-        let (column_expr, correct_operator, scalar_expr) =
-            match rewrite_expr_to_prunable(column_expr, correct_operator, scalar_expr) {
-                Ok(ret) => ret,
-                Err(e) => return Err(e),
-            };
+        let df_schema = DFSchema::try_from(schema.clone())?;
+        let (column_expr, correct_operator, scalar_expr) = match rewrite_expr_to_prunable(
+            column_expr,
+            correct_operator,
+            scalar_expr,
+            df_schema,
+        ) {
+            Ok(ret) => ret,
+            Err(e) => return Err(e),
+        };

Review Comment:
   update manually



-- 
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


[GitHub] [arrow-datafusion] liukun4515 commented on pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
liukun4515 commented on PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#issuecomment-1244905928

   Thanks for your careful review. @alamb 


-- 
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


[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #3422: support cast/try_cast in prune with signed integer and decimal

Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #3422:
URL: https://github.com/apache/arrow-datafusion/pull/3422#discussion_r967635359


##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -487,6 +492,7 @@ fn rewrite_expr_to_prunable(
     column_expr: &Expr,
     op: Operator,
     scalar_expr: &Expr,
+    schema: DFSchema,

Review Comment:
   Could you please update the list of rules in the docstring of this function to include the new logic for casts?



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -552,6 +577,25 @@ fn is_compare_op(op: Operator) -> bool {
     )
 }
 
+// The pruning logic is based on the comparing the min/max bounds.
+// Must make sure the two type has order.
+// For example, casts from string to numbers is not correct.
+// Because the "13" is less than "3" with UTF8 comparison order.
+fn support_type_for_prune(from_type: &DataType, to_type: &DataType) -> bool {
+    // TODO: support other data type

Review Comment:
   I think it would be good to file a follow on ticket listing the other types (you did so in https://github.com/apache/arrow-datafusion/issues/3414#issuecomment-1242659478)



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -429,11 +429,16 @@ impl<'a> PruningExpressionBuilder<'a> {
                 }
             };
 
-        let (column_expr, correct_operator, scalar_expr) =
-            match rewrite_expr_to_prunable(column_expr, correct_operator, scalar_expr) {
-                Ok(ret) => ret,
-                Err(e) => return Err(e),
-            };
+        let df_schema = DFSchema::try_from(schema.clone())?;
+        let (column_expr, correct_operator, scalar_expr) = match rewrite_expr_to_prunable(
+            column_expr,
+            correct_operator,
+            scalar_expr,
+            df_schema,
+        ) {
+            Ok(ret) => ret,
+            Err(e) => return Err(e),
+        };

Review Comment:
   Very minor suggestion (can do this as a follow on too)
   
   ```suggestion
           let (column_expr, correct_operator, scalar_expr) = rewrite_expr_to_prunable(
               column_expr,
               correct_operator,
               scalar_expr,
               df_schema,
           )?;
   ```



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -1995,4 +2039,66 @@ mod tests {
         let result = p.prune(&statistics).unwrap();
         assert_eq!(result, expected_ret);
     }
+
+    #[test]
+    fn test_rewrite_expr_to_prunable() {
+        let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
+        let df_schema = DFSchema::try_from(schema).unwrap();
+        // column op lit
+        let left_input = col("a");
+        let right_input = lit(ScalarValue::Int32(Some(12)));
+        let (result_left, _, result_right) = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Eq,
+            &right_input,
+            df_schema.clone(),
+        )
+        .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // cast op lit
+        let left_input = cast(col("a"), DataType::Decimal128(20, 3));
+        let right_input = lit(ScalarValue::Decimal128(Some(12), 20, 3));
+        let (result_left, _, result_right) = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Gt,
+            &right_input,
+            df_schema.clone(),
+        )
+        .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // try_cast op lit
+        let left_input = try_cast(col("a"), DataType::Int64);
+        let right_input = lit(ScalarValue::Int64(Some(12)));
+        let (result_left, _, result_right) =
+            rewrite_expr_to_prunable(&left_input, Operator::Gt, &right_input, df_schema)
+                .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // TODO: add test for other case and op

Review Comment:
   Rather than extending this test for other ops, I think we should add tests which actually run the pruning expressions on values (see my other comments).
   
   Would it be helpful for me to write some more tests? I can do so tomorrow morning 



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -552,6 +577,25 @@ fn is_compare_op(op: Operator) -> bool {
     )
 }
 
+// The pruning logic is based on the comparing the min/max bounds.
+// Must make sure the two type has order.
+// For example, casts from string to numbers is not correct.

Review Comment:
   πŸ‘  thank you for these comments



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -495,21 +501,40 @@ fn rewrite_expr_to_prunable(
     }
 
     match column_expr {
+        // col op lit()
+        Expr::Column(_) => Ok((column_expr.clone(), op, scalar_expr.clone())),
         // `cast(col) op lit()`
         Expr::Cast { expr, data_type } => {
-            let (left, op, right) = rewrite_expr_to_prunable(expr, op, scalar_expr)?;
-            Ok((cast(left, data_type.clone()), op, right))
+            let from_type = expr.get_type(&schema)?;
+            if support_type_for_prune(&from_type, data_type) {
+                let (left, op, right) =
+                    rewrite_expr_to_prunable(expr, op, scalar_expr, schema)?;
+                Ok((cast(left, data_type.clone()), op, right))
+            } else {
+                return Err(DataFusionError::Plan(format!(
+                    "Cast with from type {} to type {} is not supported",
+                    from_type, data_type
+                )));
+            }

Review Comment:
   We could probably avoid the duplication of the error generation by moving the error return into `support_type_for_prune` like:
   
   ```suggestion
               verify_support_type_for_prune(&from_type, data_type)?;
               let (left, op, right) = rewrite_expr_to_prunable(expr, op, scalar_expr)?;
               Ok((cast(left, data_type.clone()), op, right))
   ```
   
   This is not needed for correctness, just a code structure suggestion
   
   



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -1995,4 +2039,66 @@ mod tests {
         let result = p.prune(&statistics).unwrap();
         assert_eq!(result, expected_ret);
     }
+
+    #[test]
+    fn test_rewrite_expr_to_prunable() {
+        let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
+        let df_schema = DFSchema::try_from(schema).unwrap();
+        // column op lit
+        let left_input = col("a");
+        let right_input = lit(ScalarValue::Int32(Some(12)));
+        let (result_left, _, result_right) = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Eq,
+            &right_input,
+            df_schema.clone(),
+        )
+        .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // cast op lit
+        let left_input = cast(col("a"), DataType::Decimal128(20, 3));
+        let right_input = lit(ScalarValue::Decimal128(Some(12), 20, 3));
+        let (result_left, _, result_right) = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Gt,
+            &right_input,
+            df_schema.clone(),
+        )
+        .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // try_cast op lit
+        let left_input = try_cast(col("a"), DataType::Int64);
+        let right_input = lit(ScalarValue::Int64(Some(12)));
+        let (result_left, _, result_right) =
+            rewrite_expr_to_prunable(&left_input, Operator::Gt, &right_input, df_schema)
+                .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // TODO: add test for other case and op
+    }
+
+    #[test]
+    fn test_rewrite_expr_to_prunable_error() {
+        // cast string value to numeric value
+        // this cast is not supported
+        let schema = Schema::new(vec![Field::new("a", DataType::Utf8, true)]);
+        let df_schema = DFSchema::try_from(schema).unwrap();
+        let left_input = cast(col("a"), DataType::Int64);
+        let right_input = lit(ScalarValue::Int64(Some(12)));
+        let result = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Gt,
+            &right_input,
+            df_schema.clone(),
+        );
+        assert!(result.is_err());
+        // other expr
+        let left_input = is_null(col("a"));
+        let right_input = lit(ScalarValue::Int64(Some(12)));
+        let result =
+            rewrite_expr_to_prunable(&left_input, Operator::Gt, &right_input, df_schema);
+        assert!(result.is_err());
+    }
 }

Review Comment:
   I think we should add some tests that exercise the pruning code with actual values too
   
   For example, perhaps you can take `prune_int32_cast` from https://github.com/apache/arrow-datafusion/pull/3378?



##########
datafusion/core/src/physical_optimizer/pruning.rs:
##########
@@ -1995,4 +2039,66 @@ mod tests {
         let result = p.prune(&statistics).unwrap();
         assert_eq!(result, expected_ret);
     }
+
+    #[test]
+    fn test_rewrite_expr_to_prunable() {
+        let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
+        let df_schema = DFSchema::try_from(schema).unwrap();
+        // column op lit
+        let left_input = col("a");
+        let right_input = lit(ScalarValue::Int32(Some(12)));
+        let (result_left, _, result_right) = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Eq,
+            &right_input,
+            df_schema.clone(),
+        )
+        .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // cast op lit
+        let left_input = cast(col("a"), DataType::Decimal128(20, 3));
+        let right_input = lit(ScalarValue::Decimal128(Some(12), 20, 3));
+        let (result_left, _, result_right) = rewrite_expr_to_prunable(
+            &left_input,
+            Operator::Gt,
+            &right_input,
+            df_schema.clone(),
+        )
+        .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // try_cast op lit
+        let left_input = try_cast(col("a"), DataType::Int64);
+        let right_input = lit(ScalarValue::Int64(Some(12)));
+        let (result_left, _, result_right) =
+            rewrite_expr_to_prunable(&left_input, Operator::Gt, &right_input, df_schema)
+                .unwrap();
+        assert_eq!(result_left, left_input);
+        assert_eq!(result_right, right_input);
+        // TODO: add test for other case and op
+    }
+
+    #[test]
+    fn test_rewrite_expr_to_prunable_error() {
+        // cast string value to numeric value
+        // this cast is not supported

Review Comment:
   πŸ‘ 



-- 
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