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



##########
File path: datafusion/tests/parquet_pruning.rs
##########
@@ -177,6 +178,229 @@ async fn prune_disabled() {
     );
 }
 
+#[tokio::test]
+async fn prune_int32_lt() {
+    let (expected_errors, expected_row_group_pruned, expected_results) =
+        (Some(0), Some(1), 11);
+
+    // resulrt of sql "SELECT * FROM t where i < 1" is same as
+    // "SELECT * FROM t where -i > -1"
+    let output = ContextWithParquet::new(Scenario::Int32)
+        .await
+        .query("SELECT * FROM t where i < 1")
+        .await;
+
+    println!("{}", output.description());
+    // This should prune out groups without error
+    assert_eq!(output.predicate_evaluation_errors(), expected_errors);
+    assert_eq!(output.row_groups_pruned(), expected_row_group_pruned);
+    assert_eq!(
+        output.result_rows,
+        expected_results,
+        "{}",
+        output.description()
+    );
+
+    let output = ContextWithParquet::new(Scenario::Int32)
+        .await
+        .query("SELECT * FROM t where -i > -1")
+        .await;
+
+    println!("{}", output.description());
+    // This should prune out groups without error
+    assert_eq!(output.predicate_evaluation_errors(), expected_errors);
+    assert_eq!(output.row_groups_pruned(), expected_row_group_pruned);
+    assert_eq!(
+        output.result_rows,
+        expected_results,
+        "{}",
+        output.description()
+    );
+}
+
+#[tokio::test]
+async fn prune_int32_eq() {
+    // resulrt of sql "SELECT * FROM t where i = 1"
+    let output = ContextWithParquet::new(Scenario::Int32)
+        .await
+        .query("SELECT * FROM t where i = 1")
+        .await;
+
+    println!("{}", output.description());
+    // This should prune out groups without error
+    assert_eq!(output.predicate_evaluation_errors(), Some(0));
+    assert_eq!(output.row_groups_pruned(), Some(3));

Review comment:
       👍  I forgot initially that `-4..1` doesn't actually include `1` (it is 
`-4, -3, -2, -1, 0` and thus should be pruned) 👍 

##########
File path: datafusion/tests/parquet_pruning.rs
##########
@@ -177,6 +178,229 @@ async fn prune_disabled() {
     );
 }
 
+#[tokio::test]
+async fn prune_int32_lt() {
+    let (expected_errors, expected_row_group_pruned, expected_results) =
+        (Some(0), Some(1), 11);
+
+    // resulrt of sql "SELECT * FROM t where i < 1" is same as
+    // "SELECT * FROM t where -i > -1"
+    let output = ContextWithParquet::new(Scenario::Int32)
+        .await
+        .query("SELECT * FROM t where i < 1")
+        .await;
+
+    println!("{}", output.description());
+    // This should prune out groups without error
+    assert_eq!(output.predicate_evaluation_errors(), expected_errors);
+    assert_eq!(output.row_groups_pruned(), expected_row_group_pruned);
+    assert_eq!(
+        output.result_rows,
+        expected_results,
+        "{}",
+        output.description()
+    );
+
+    let output = ContextWithParquet::new(Scenario::Int32)
+        .await
+        .query("SELECT * FROM t where -i > -1")
+        .await;
+
+    println!("{}", output.description());
+    // This should prune out groups without error
+    assert_eq!(output.predicate_evaluation_errors(), expected_errors);
+    assert_eq!(output.row_groups_pruned(), expected_row_group_pruned);
+    assert_eq!(
+        output.result_rows,
+        expected_results,
+        "{}",
+        output.description()
+    );
+}
+
+#[tokio::test]
+async fn prune_int32_eq() {
+    // resulrt of sql "SELECT * FROM t where i = 1"
+    let output = ContextWithParquet::new(Scenario::Int32)
+        .await
+        .query("SELECT * FROM t where i = 1")
+        .await;
+
+    println!("{}", output.description());
+    // This should prune out groups without error
+    assert_eq!(output.predicate_evaluation_errors(), Some(0));
+    assert_eq!(output.row_groups_pruned(), Some(3));
+    assert_eq!(output.result_rows, 1, "{}", output.description());
+}
+
+#[tokio::test]
+async fn prune_int32_scalar_fun_and_eq() {
+    // resulrt of sql "SELECT * FROM t where abs(i) = 1 and i = 1"
+    // only use "i = 1" to prune
+    let output = ContextWithParquet::new(Scenario::Int32)
+        .await
+        .query("SELECT * FROM t where abs(i) = 1  and i = 1")
+        .await;
+
+    println!("{}", output.description());
+    // This should prune out groups without error
+    assert_eq!(output.predicate_evaluation_errors(), Some(0));
+    assert_eq!(output.row_groups_pruned(), Some(3));
+    assert_eq!(output.result_rows, 1, "{}", output.description());
+}
+
+#[tokio::test]
+async fn prune_int32_scalar_fun() {
+    // resulrt of sql "SELECT * FROM t where abs(i) = 1" is not supported
+    let output = ContextWithParquet::new(Scenario::Int32)
+        .await
+        .query("SELECT * FROM t where abs(i) = 1")
+        .await;
+
+    println!("{}", output.description());
+    // This should prune out groups with error, because there is not col to
+    // prune the row groups.
+    assert_eq!(output.predicate_evaluation_errors(), Some(1));
+    assert_eq!(output.row_groups_pruned(), Some(0));
+    assert_eq!(output.result_rows, 3, "{}", output.description());
+}
+
+#[tokio::test]
+async fn prune_int32_complex_expr() {
+    // resulrt of sql "SELECT * FROM t where i+1 = 1" is not supported
+    let output = ContextWithParquet::new(Scenario::Int32)
+        .await
+        .query("SELECT * FROM t where i+1 = 1")
+        .await;
+
+    println!("{}", output.description());
+    // This should prune out groups with error, because there is not col to

Review comment:
       yeah, and in the future we can add some more sophistication to the 
predicate builder to handle these types of monotonic functions (e.g. `+1`)

##########
File path: datafusion/src/physical_optimizer/pruning.rs
##########
@@ -386,6 +387,12 @@ impl<'a> PruningExpressionBuilder<'a> {
                     ));
                 }
             };
+
+        let (column_expr, correct_operator, scalar_expr) =

Review comment:
       I think this is a nice change and improves readability




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to