Skip to content

Commit 39d14e9

Browse files
committed
Add test for not simplify
1 parent 37d7d05 commit 39d14e9

3 files changed

Lines changed: 72 additions & 11 deletions

File tree

datafusion/core/tests/parquet/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ impl ContextWithParquet {
228228
let file = match unit {
229229
Unit::RowGroup(row_per_group) => {
230230
config = config.with_parquet_bloom_filter_pruning(true);
231+
config.options_mut().execution.parquet.pushdown_filters = true;
231232
make_test_file_rg(scenario, row_per_group, custom_schema, custom_batches)
232233
.await
233234
}

datafusion/core/tests/parquet/row_group_pruning.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1754,8 +1754,8 @@ async fn test_limit_pruning() -> datafusion_common::error::Result<()> {
17541754
.with_expected_rows(2)
17551755
.with_pruned_files(Some(0))
17561756
.with_matched_by_stats(Some(4))
1757-
.with_pruned_by_stats(Some(1))
1758-
.with_limit_pruned_row_groups(Some(2))
1757+
.with_pruned_by_stats(Some(1))
1758+
.with_limit_pruned_row_groups(Some(3))
17591759
.test_row_group_prune_with_custom_data(schema, batches, 2)
17601760
.await;
17611761

datafusion/physical-expr/src/simplifier/not.rs

Lines changed: 69 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ pub(crate) fn simplify_not_expr(
4949
// Handle NOT(NOT(expr)) -> expr (double negation elimination)
5050
if let Some(inner_not) = inner_expr.as_any().downcast_ref::<NotExpr>() {
5151
// Recursively simplify the inner expression
52-
return simplify_not_expr_recursive(Arc::clone(inner_not.arg()), schema);
52+
let simplified =
53+
simplify_not_expr_recursive(Arc::clone(inner_not.arg()), schema)?;
54+
// We eliminated double negation, so always return transformed=true
55+
return Ok(Transformed::yes(simplified.data));
5356
}
5457

5558
// Handle NOT(literal) -> !literal
@@ -65,12 +68,19 @@ pub(crate) fn simplify_not_expr(
6568
// Handle NOT(binary_expr) where we can flip the operator
6669
if let Some(binary_expr) = inner_expr.as_any().downcast_ref::<BinaryExpr>() {
6770
if let Some(negated_op) = negate_operator(binary_expr.op()) {
71+
// Recursively simplify the left and right expressions first
72+
let left_simplified =
73+
simplify_not_expr_recursive(Arc::clone(binary_expr.left()), schema)?;
74+
let right_simplified =
75+
simplify_not_expr_recursive(Arc::clone(binary_expr.right()), schema)?;
76+
6877
let new_binary = Arc::new(BinaryExpr::new(
69-
Arc::clone(binary_expr.left()),
78+
left_simplified.data,
7079
negated_op,
71-
Arc::clone(binary_expr.right()),
80+
right_simplified.data,
7281
));
73-
return simplify_not_expr_recursive(new_binary, schema);
82+
// We flipped the operator, so always return transformed=true
83+
return Ok(Transformed::yes(new_binary));
7484
}
7585

7686
// Handle De Morgan's laws for AND/OR
@@ -200,7 +210,7 @@ mod tests {
200210
let inner_not = Arc::new(NotExpr::new(inner_expr.clone()));
201211
let double_not = Arc::new(NotExpr::new(inner_not));
202212

203-
let result = simplify_not_expr(double_not, &schema)?;
213+
let result = simplify_not_expr_recursive(double_not, &schema)?;
204214

205215
assert!(result.transformed);
206216
// Should be simplified back to the original b > 5
@@ -225,7 +235,7 @@ mod tests {
225235

226236
// NOT(FALSE) -> TRUE
227237
let not_false = Arc::new(NotExpr::new(lit(ScalarValue::Boolean(Some(false)))));
228-
let result = simplify_not_expr(not_false, &schema)?;
238+
let result = simplify_not_expr_recursive(not_false, &schema)?;
229239
assert!(result.transformed);
230240

231241
if let Some(literal) = result.data.as_any().downcast_ref::<Literal>() {
@@ -249,7 +259,7 @@ mod tests {
249259
));
250260
let not_eq = Arc::new(NotExpr::new(eq_expr));
251261

252-
let result = simplify_not_expr(not_eq, &schema)?;
262+
let result = simplify_not_expr_recursive(not_eq, &schema)?;
253263
assert!(result.transformed);
254264

255265
if let Some(binary) = result.data.as_any().downcast_ref::<BinaryExpr>() {
@@ -273,7 +283,7 @@ mod tests {
273283
));
274284
let not_and = Arc::new(NotExpr::new(and_expr));
275285

276-
let result = simplify_not_expr(not_and, &schema)?;
286+
let result = simplify_not_expr_recursive(not_and, &schema)?;
277287
assert!(result.transformed);
278288

279289
if let Some(binary) = result.data.as_any().downcast_ref::<BinaryExpr>() {
@@ -300,7 +310,7 @@ mod tests {
300310
));
301311
let not_or = Arc::new(NotExpr::new(or_expr));
302312

303-
let result = simplify_not_expr(not_or, &schema)?;
313+
let result = simplify_not_expr_recursive(not_or, &schema)?;
304314
assert!(result.transformed);
305315

306316
if let Some(binary) = result.data.as_any().downcast_ref::<BinaryExpr>() {
@@ -314,4 +324,54 @@ mod tests {
314324

315325
Ok(())
316326
}
327+
328+
#[test]
329+
fn test_demorgans_with_comparison_simplification() -> Result<()> {
330+
let schema = test_schema();
331+
332+
// NOT(b = 1 AND b = 2) -> b != 1 OR b != 2
333+
// This tests the combination of De Morgan's law and operator negation
334+
let eq1 = Arc::new(BinaryExpr::new(
335+
col("b", &schema)?,
336+
Operator::Eq,
337+
lit(ScalarValue::Int32(Some(1))),
338+
));
339+
let eq2 = Arc::new(BinaryExpr::new(
340+
col("b", &schema)?,
341+
Operator::Eq,
342+
lit(ScalarValue::Int32(Some(2))),
343+
));
344+
let and_expr = Arc::new(BinaryExpr::new(eq1, Operator::And, eq2));
345+
let not_and = Arc::new(NotExpr::new(and_expr));
346+
347+
let result = simplify_not_expr_recursive(not_and, &schema)?;
348+
assert!(result.transformed, "Expression should be transformed");
349+
350+
// Verify the result is an OR expression
351+
if let Some(or_binary) = result.data.as_any().downcast_ref::<BinaryExpr>() {
352+
assert_eq!(or_binary.op(), &Operator::Or, "Top level should be OR");
353+
354+
// Verify left side is b != 1
355+
if let Some(left_binary) =
356+
or_binary.left().as_any().downcast_ref::<BinaryExpr>()
357+
{
358+
assert_eq!(left_binary.op(), &Operator::NotEq, "Left should be NotEq");
359+
} else {
360+
panic!("Expected left to be a binary expression with !=");
361+
}
362+
363+
// Verify right side is b != 2
364+
if let Some(right_binary) =
365+
or_binary.right().as_any().downcast_ref::<BinaryExpr>()
366+
{
367+
assert_eq!(right_binary.op(), &Operator::NotEq, "Right should be NotEq");
368+
} else {
369+
panic!("Expected right to be a binary expression with !=");
370+
}
371+
} else {
372+
panic!("Expected binary OR expression result");
373+
}
374+
375+
Ok(())
376+
}
317377
}

0 commit comments

Comments
 (0)