alamb commented on a change in pull request #1376:
URL: https://github.com/apache/arrow-datafusion/pull/1376#discussion_r763493951
##########
File path: datafusion/src/optimizer/simplify_expressions.rs
##########
@@ -1406,62 +1352,40 @@ mod tests {
else_expr: Some(Box::new(col("c2"))),
}
);
+ }
- Ok(())
+ fn lit_true() -> Expr {
Review comment:
No this is a great point. I will change to use `lit(true)` and
`lit(false)` -- that is much better I think
##########
File path: datafusion/src/optimizer/simplify_expressions.rs
##########
@@ -1191,212 +1178,171 @@ mod tests {
test_evaluate_with_start_time(input_expr, expected_expr, &Utc::now())
}
- #[test]
- fn simplify_expr_not_not() -> Result<()> {
- let schema = expr_test_schema();
- let mut rewriter = Simplifier {
- schemas: vec![&schema],
- };
+ // ------------------------------
+ // ----- Simplifier tests -------
+ // ------------------------------
- assert_eq!(
- (col("c2").not().not().not()).rewrite(&mut rewriter)?,
- col("c2").not(),
- );
+ // TODO rename to simplify
+ fn do_simplify(expr: Expr) -> Expr {
Review comment:
I actually have plans to rename it in a follow on PR (b/c I want to
remove `simpify`) -- you can see what I have in mind here:
https://github.com/apache/arrow-datafusion/pull/1401
##########
File path: datafusion/src/optimizer/simplify_expressions.rs
##########
@@ -1191,212 +1178,171 @@ mod tests {
test_evaluate_with_start_time(input_expr, expected_expr, &Utc::now())
}
- #[test]
- fn simplify_expr_not_not() -> Result<()> {
- let schema = expr_test_schema();
- let mut rewriter = Simplifier {
- schemas: vec![&schema],
- };
+ // ------------------------------
+ // ----- Simplifier tests -------
+ // ------------------------------
- assert_eq!(
- (col("c2").not().not().not()).rewrite(&mut rewriter)?,
- col("c2").not(),
- );
+ // TODO rename to simplify
+ fn do_simplify(expr: Expr) -> Expr {
+ let schema = expr_test_schema();
+ let mut rewriter = Simplifier::new(vec![&schema]);
+ expr.rewrite(&mut rewriter).expect("expected to simplify")
+ }
- Ok(())
+ fn expr_test_schema() -> DFSchemaRef {
+ Arc::new(
+ DFSchema::new(vec![
+ DFField::new(None, "c1", DataType::Utf8, true),
+ DFField::new(None, "c2", DataType::Boolean, true),
+ ])
+ .unwrap(),
+ )
}
#[test]
- fn simplify_expr_null_comparison() -> Result<()> {
- let schema = expr_test_schema();
- let mut rewriter = Simplifier {
- schemas: vec![&schema],
- };
+ fn simplify_expr_not_not() {
+ assert_eq!(do_simplify(col("c2").not().not().not()), col("c2").not(),);
+ }
+ #[test]
+ fn simplify_expr_null_comparison() {
// x = null is always null
assert_eq!(
- (lit(true).eq(lit(ScalarValue::Boolean(None)))).rewrite(&mut
rewriter)?,
+ do_simplify(lit(true).eq(lit(ScalarValue::Boolean(None)))),
lit(ScalarValue::Boolean(None)),
);
// null != null is always null
assert_eq!(
-
(lit(ScalarValue::Boolean(None)).not_eq(lit(ScalarValue::Boolean(None))))
- .rewrite(&mut rewriter)?,
+ do_simplify(
+
lit(ScalarValue::Boolean(None)).not_eq(lit(ScalarValue::Boolean(None)))
+ ),
lit(ScalarValue::Boolean(None)),
);
// x != null is always null
assert_eq!(
- (col("c2").not_eq(lit(ScalarValue::Boolean(None)))).rewrite(&mut
rewriter)?,
+ do_simplify(col("c2").not_eq(lit(ScalarValue::Boolean(None)))),
lit(ScalarValue::Boolean(None)),
);
// null = x is always null
assert_eq!(
- (lit(ScalarValue::Boolean(None)).eq(col("c2"))).rewrite(&mut
rewriter)?,
+ do_simplify(lit(ScalarValue::Boolean(None)).eq(col("c2"))),
lit(ScalarValue::Boolean(None)),
);
-
- Ok(())
}
#[test]
- fn simplify_expr_eq() -> Result<()> {
+ fn simplify_expr_eq() {
let schema = expr_test_schema();
- let mut rewriter = Simplifier {
- schemas: vec![&schema],
- };
-
- assert_eq!(col("c2").get_type(&schema)?, DataType::Boolean);
+ assert_eq!(col("c2").get_type(&schema).unwrap(), DataType::Boolean);
// true = ture -> true
- assert_eq!((lit(true).eq(lit(true))).rewrite(&mut rewriter)?,
lit(true),);
+ assert_eq!(do_simplify(lit(true).eq(lit(true))), lit(true));
// true = false -> false
- assert_eq!(
- (lit(true).eq(lit(false))).rewrite(&mut rewriter)?,
- lit(false),
- );
+ assert_eq!(do_simplify(lit(true).eq(lit(false))), lit(false),);
// c2 = true -> c2
- assert_eq!((col("c2").eq(lit(true))).rewrite(&mut rewriter)?,
col("c2"),);
+ assert_eq!(do_simplify(col("c2").eq(lit(true))), col("c2"));
// c2 = false => !c2
- assert_eq!(
- (col("c2").eq(lit(false))).rewrite(&mut rewriter)?,
- col("c2").not(),
- );
-
- Ok(())
+ assert_eq!(do_simplify(col("c2").eq(lit(false))), col("c2").not(),);
}
#[test]
- fn simplify_expr_eq_skip_nonboolean_type() -> Result<()> {
+ fn simplify_expr_eq_skip_nonboolean_type() {
let schema = expr_test_schema();
- let mut rewriter = Simplifier {
- schemas: vec![&schema],
- };
- // When one of the operand is not of boolean type, folding the other
boolean constant will
- // change return type of expression to non-boolean.
+ // When one of the operand is not of boolean type, folding the
+ // other boolean constant will change return type of
+ // expression to non-boolean.
//
// Make sure c1 column to be used in tests is not boolean type
- assert_eq!(col("c1").get_type(&schema)?, DataType::Utf8);
+ assert_eq!(col("c1").get_type(&schema).unwrap(), DataType::Utf8);
Review comment:
So I think that plan would error at runtime (because `"d"` is defined to
be `DataTypre::UInt32` and if you try to do `UInt32 != Bool` you get an error:
```
❯ select 1 != true;
Plan("'Int64 != Boolean' can't be evaluated because there isn't a common
type to coerce the types to")
```
Though it does work if we have an explicit `CAST`:
```
❯ select 1 != cast(true as int);
+------------------------------------------+
| Int64(1) != CAST(Boolean(true) AS Int32) |
+------------------------------------------+
| false |
+------------------------------------------+
1 row in set. Query took 0.003 seconds.
```
I guess I was thinking the plan tests are for ensuring that `simplify` is
being called correctly on the expressions in the plan, rather than testing the
various `Expr` corner cases of the simplification logic itself
##########
File path: datafusion/src/optimizer/simplify_expressions.rs
##########
@@ -785,180 +786,166 @@ mod tests {
use crate::physical_plan::udf::ScalarUDF;
#[test]
- fn test_simplify_or_true() -> Result<()> {
- let expr_a = col("c").or(lit(true));
- let expr_b = lit(true).or(col("c"));
+ fn test_simplify_or_true() {
+ let expr_a = col("c2").or(lit(true));
+ let expr_b = lit(true).or(col("c2"));
let expected = lit(true);
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_false() -> Result<()> {
- let expr_a = lit(false).or(col("c"));
- let expr_b = col("c").or(lit(false));
- let expected = col("c");
+ fn test_simplify_or_false() {
+ let expr_a = lit(false).or(col("c2"));
+ let expr_b = col("c2").or(lit(false));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_same() -> Result<()> {
- let expr = col("c").or(col("c"));
- let expected = col("c");
+ fn test_simplify_or_same() {
+ let expr = col("c2").or(col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_false() -> Result<()> {
- let expr_a = lit(false).and(col("c"));
- let expr_b = col("c").and(lit(false));
+ fn test_simplify_and_false() {
+ let expr_a = lit(false).and(col("c2"));
+ let expr_b = col("c2").and(lit(false));
let expected = lit(false);
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_same() -> Result<()> {
- let expr = col("c").and(col("c"));
- let expected = col("c");
+ fn test_simplify_and_same() {
+ let expr = col("c2").and(col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_true() -> Result<()> {
- let expr_a = lit(true).and(col("c"));
- let expr_b = col("c").and(lit(true));
- let expected = col("c");
+ fn test_simplify_and_true() {
+ let expr_a = lit(true).and(col("c2"));
+ let expr_b = col("c2").and(lit(true));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_multiply_by_one() -> Result<()> {
- let expr_a = binary_expr(col("c"), Operator::Multiply, lit(1));
- let expr_b = binary_expr(lit(1), Operator::Multiply, col("c"));
- let expected = col("c");
+ fn test_simplify_multiply_by_one() {
Review comment:
I tried that, and it turns out that rewrite rule is not implemented LOL
👍
```diff
diff --git a/datafusion/src/optimizer/simplify_expressions.rs
b/datafusion/src/optimizer/simplify_expressions.rs
index 3c4af838b..68d45c446 100644
--- a/datafusion/src/optimizer/simplify_expressions.rs
+++ b/datafusion/src/optimizer/simplify_expressions.rs
@@ -851,6 +851,16 @@ mod tests {
assert_eq!(simplify(&expr_b), expected);
}
+ #[test]
+ fn test_simplify_multiply_by_zero() {
+ let expr_a = binary_expr(col("c2"), Operator::Multiply, lit(0));
+ let expr_b = binary_expr(lit(0), Operator::Multiply, col("c2"));
+ let expected = lit(0);
+
+ assert_eq!(simplify(&expr_a), expected);
+ assert_eq!(simplify(&expr_b), expected);
+ }
+
#[test]
fn test_simplify_divide_by_one() {
let expr = binary_expr(col("c2"), Operator::Divide, lit(1));
```
Resulted in
```
---- optimizer::simplify_expressions::tests::test_simplify_multiply_by_zero
stdout ----
thread
'optimizer::simplify_expressions::tests::test_simplify_multiply_by_zero'
panicked at 'assertion failed: `(left == right)`
left: `#c2 * Int32(0)`,
right: `Int32(0)`', datafusion/src/optimizer/simplify_expressions.rs:860:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
##########
File path: datafusion/src/optimizer/simplify_expressions.rs
##########
@@ -785,180 +786,166 @@ mod tests {
use crate::physical_plan::udf::ScalarUDF;
#[test]
- fn test_simplify_or_true() -> Result<()> {
- let expr_a = col("c").or(lit(true));
- let expr_b = lit(true).or(col("c"));
+ fn test_simplify_or_true() {
+ let expr_a = col("c2").or(lit(true));
+ let expr_b = lit(true).or(col("c2"));
let expected = lit(true);
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_false() -> Result<()> {
- let expr_a = lit(false).or(col("c"));
- let expr_b = col("c").or(lit(false));
- let expected = col("c");
+ fn test_simplify_or_false() {
+ let expr_a = lit(false).or(col("c2"));
+ let expr_b = col("c2").or(lit(false));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_same() -> Result<()> {
- let expr = col("c").or(col("c"));
- let expected = col("c");
+ fn test_simplify_or_same() {
+ let expr = col("c2").or(col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_false() -> Result<()> {
- let expr_a = lit(false).and(col("c"));
- let expr_b = col("c").and(lit(false));
+ fn test_simplify_and_false() {
+ let expr_a = lit(false).and(col("c2"));
+ let expr_b = col("c2").and(lit(false));
let expected = lit(false);
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_same() -> Result<()> {
- let expr = col("c").and(col("c"));
- let expected = col("c");
+ fn test_simplify_and_same() {
+ let expr = col("c2").and(col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_true() -> Result<()> {
- let expr_a = lit(true).and(col("c"));
- let expr_b = col("c").and(lit(true));
- let expected = col("c");
+ fn test_simplify_and_true() {
+ let expr_a = lit(true).and(col("c2"));
+ let expr_b = col("c2").and(lit(true));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_multiply_by_one() -> Result<()> {
- let expr_a = binary_expr(col("c"), Operator::Multiply, lit(1));
- let expr_b = binary_expr(lit(1), Operator::Multiply, col("c"));
- let expected = col("c");
+ fn test_simplify_multiply_by_one() {
+ let expr_a = binary_expr(col("c2"), Operator::Multiply, lit(1));
+ let expr_b = binary_expr(lit(1), Operator::Multiply, col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_divide_by_one() -> Result<()> {
- let expr = binary_expr(col("c"), Operator::Divide, lit(1));
- let expected = col("c");
+ fn test_simplify_divide_by_one() {
+ let expr = binary_expr(col("c2"), Operator::Divide, lit(1));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_divide_by_same() -> Result<()> {
- let expr = binary_expr(col("c"), Operator::Divide, col("c"));
+ fn test_simplify_divide_by_same() {
+ let expr = binary_expr(col("c2"), Operator::Divide, col("c2"));
let expected = lit(1);
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_simple_and() -> Result<()> {
+ fn test_simplify_simple_and() {
// (c > 5) AND (c > 5)
- let expr = (col("c").gt(lit(5))).and(col("c").gt(lit(5)));
- let expected = col("c").gt(lit(5));
+ let expr = (col("c2").gt(lit(5))).and(col("c2").gt(lit(5)));
+ let expected = col("c2").gt(lit(5));
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_composed_and() -> Result<()> {
+ fn test_simplify_composed_and() {
// ((c > 5) AND (d < 6)) AND (c > 5)
let expr = binary_expr(
- binary_expr(col("c").gt(lit(5)), Operator::And,
col("d").lt(lit(6))),
+ binary_expr(col("c2").gt(lit(5)), Operator::And,
col("d").lt(lit(6))),
Operator::And,
- col("c").gt(lit(5)),
+ col("c2").gt(lit(5)),
);
let expected =
- binary_expr(col("c").gt(lit(5)), Operator::And,
col("d").lt(lit(6)));
+ binary_expr(col("c2").gt(lit(5)), Operator::And,
col("d").lt(lit(6)));
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_negated_and() -> Result<()> {
+ fn test_simplify_negated_and() {
// (c > 5) AND !(c > 5) -- can't remove
let expr = binary_expr(
- col("c").gt(lit(5)),
+ col("c2").gt(lit(5)),
Operator::And,
- Expr::not(col("c").gt(lit(5))),
+ Expr::not(col("c2").gt(lit(5))),
);
let expected = expr.clone();
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_and() -> Result<()> {
+ fn test_simplify_or_and() {
// (c > 5) OR ((d < 6) AND (c > 5) -- can remove
let expr = binary_expr(
- col("c").gt(lit(5)),
+ col("c2").gt(lit(5)),
Operator::Or,
- binary_expr(col("d").lt(lit(6)), Operator::And,
col("c").gt(lit(5))),
+ binary_expr(col("d").lt(lit(6)), Operator::And,
col("c2").gt(lit(5))),
);
- let expected = col("c").gt(lit(5));
+ let expected = col("c2").gt(lit(5));
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_and_false() -> Result<()> {
- let expr =
- binary_expr(lit(ScalarValue::Boolean(None)), Operator::And,
lit(false));
+ fn test_simplify_and_and_false() {
Review comment:
Good 👀 👍
##########
File path: datafusion/src/optimizer/simplify_expressions.rs
##########
@@ -785,180 +786,166 @@ mod tests {
use crate::physical_plan::udf::ScalarUDF;
#[test]
- fn test_simplify_or_true() -> Result<()> {
- let expr_a = col("c").or(lit(true));
- let expr_b = lit(true).or(col("c"));
+ fn test_simplify_or_true() {
+ let expr_a = col("c2").or(lit(true));
+ let expr_b = lit(true).or(col("c2"));
let expected = lit(true);
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_false() -> Result<()> {
- let expr_a = lit(false).or(col("c"));
- let expr_b = col("c").or(lit(false));
- let expected = col("c");
+ fn test_simplify_or_false() {
+ let expr_a = lit(false).or(col("c2"));
+ let expr_b = col("c2").or(lit(false));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_same() -> Result<()> {
- let expr = col("c").or(col("c"));
- let expected = col("c");
+ fn test_simplify_or_same() {
+ let expr = col("c2").or(col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_false() -> Result<()> {
- let expr_a = lit(false).and(col("c"));
- let expr_b = col("c").and(lit(false));
+ fn test_simplify_and_false() {
+ let expr_a = lit(false).and(col("c2"));
+ let expr_b = col("c2").and(lit(false));
let expected = lit(false);
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_same() -> Result<()> {
- let expr = col("c").and(col("c"));
- let expected = col("c");
+ fn test_simplify_and_same() {
+ let expr = col("c2").and(col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_true() -> Result<()> {
- let expr_a = lit(true).and(col("c"));
- let expr_b = col("c").and(lit(true));
- let expected = col("c");
+ fn test_simplify_and_true() {
+ let expr_a = lit(true).and(col("c2"));
+ let expr_b = col("c2").and(lit(true));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_multiply_by_one() -> Result<()> {
- let expr_a = binary_expr(col("c"), Operator::Multiply, lit(1));
- let expr_b = binary_expr(lit(1), Operator::Multiply, col("c"));
- let expected = col("c");
+ fn test_simplify_multiply_by_one() {
Review comment:
Tracking in https://github.com/apache/arrow-datafusion/issues/1406
##########
File path: datafusion/src/optimizer/simplify_expressions.rs
##########
@@ -785,180 +786,166 @@ mod tests {
use crate::physical_plan::udf::ScalarUDF;
#[test]
- fn test_simplify_or_true() -> Result<()> {
- let expr_a = col("c").or(lit(true));
- let expr_b = lit(true).or(col("c"));
+ fn test_simplify_or_true() {
+ let expr_a = col("c2").or(lit(true));
+ let expr_b = lit(true).or(col("c2"));
let expected = lit(true);
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_false() -> Result<()> {
- let expr_a = lit(false).or(col("c"));
- let expr_b = col("c").or(lit(false));
- let expected = col("c");
+ fn test_simplify_or_false() {
+ let expr_a = lit(false).or(col("c2"));
+ let expr_b = col("c2").or(lit(false));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_same() -> Result<()> {
- let expr = col("c").or(col("c"));
- let expected = col("c");
+ fn test_simplify_or_same() {
+ let expr = col("c2").or(col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_false() -> Result<()> {
- let expr_a = lit(false).and(col("c"));
- let expr_b = col("c").and(lit(false));
+ fn test_simplify_and_false() {
+ let expr_a = lit(false).and(col("c2"));
+ let expr_b = col("c2").and(lit(false));
let expected = lit(false);
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_same() -> Result<()> {
- let expr = col("c").and(col("c"));
- let expected = col("c");
+ fn test_simplify_and_same() {
+ let expr = col("c2").and(col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_true() -> Result<()> {
- let expr_a = lit(true).and(col("c"));
- let expr_b = col("c").and(lit(true));
- let expected = col("c");
+ fn test_simplify_and_true() {
+ let expr_a = lit(true).and(col("c2"));
+ let expr_b = col("c2").and(lit(true));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_multiply_by_one() -> Result<()> {
- let expr_a = binary_expr(col("c"), Operator::Multiply, lit(1));
- let expr_b = binary_expr(lit(1), Operator::Multiply, col("c"));
- let expected = col("c");
+ fn test_simplify_multiply_by_one() {
+ let expr_a = binary_expr(col("c2"), Operator::Multiply, lit(1));
+ let expr_b = binary_expr(lit(1), Operator::Multiply, col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_divide_by_one() -> Result<()> {
- let expr = binary_expr(col("c"), Operator::Divide, lit(1));
- let expected = col("c");
+ fn test_simplify_divide_by_one() {
+ let expr = binary_expr(col("c2"), Operator::Divide, lit(1));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_divide_by_same() -> Result<()> {
- let expr = binary_expr(col("c"), Operator::Divide, col("c"));
+ fn test_simplify_divide_by_same() {
+ let expr = binary_expr(col("c2"), Operator::Divide, col("c2"));
let expected = lit(1);
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_simple_and() -> Result<()> {
+ fn test_simplify_simple_and() {
// (c > 5) AND (c > 5)
- let expr = (col("c").gt(lit(5))).and(col("c").gt(lit(5)));
- let expected = col("c").gt(lit(5));
+ let expr = (col("c2").gt(lit(5))).and(col("c2").gt(lit(5)));
+ let expected = col("c2").gt(lit(5));
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_composed_and() -> Result<()> {
+ fn test_simplify_composed_and() {
// ((c > 5) AND (d < 6)) AND (c > 5)
let expr = binary_expr(
- binary_expr(col("c").gt(lit(5)), Operator::And,
col("d").lt(lit(6))),
+ binary_expr(col("c2").gt(lit(5)), Operator::And,
col("d").lt(lit(6))),
Operator::And,
- col("c").gt(lit(5)),
+ col("c2").gt(lit(5)),
);
let expected =
- binary_expr(col("c").gt(lit(5)), Operator::And,
col("d").lt(lit(6)));
+ binary_expr(col("c2").gt(lit(5)), Operator::And,
col("d").lt(lit(6)));
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_negated_and() -> Result<()> {
+ fn test_simplify_negated_and() {
Review comment:
There is no theoretical reason. I think this test is to ensure a
particular corner case of how the rewrite rule for `A and (B and A)` is
implemented.
##########
File path: datafusion/src/optimizer/simplify_expressions.rs
##########
@@ -785,180 +786,166 @@ mod tests {
use crate::physical_plan::udf::ScalarUDF;
#[test]
- fn test_simplify_or_true() -> Result<()> {
- let expr_a = col("c").or(lit(true));
- let expr_b = lit(true).or(col("c"));
+ fn test_simplify_or_true() {
+ let expr_a = col("c2").or(lit(true));
+ let expr_b = lit(true).or(col("c2"));
let expected = lit(true);
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_false() -> Result<()> {
- let expr_a = lit(false).or(col("c"));
- let expr_b = col("c").or(lit(false));
- let expected = col("c");
+ fn test_simplify_or_false() {
+ let expr_a = lit(false).or(col("c2"));
+ let expr_b = col("c2").or(lit(false));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_or_same() -> Result<()> {
- let expr = col("c").or(col("c"));
- let expected = col("c");
+ fn test_simplify_or_same() {
+ let expr = col("c2").or(col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_false() -> Result<()> {
- let expr_a = lit(false).and(col("c"));
- let expr_b = col("c").and(lit(false));
+ fn test_simplify_and_false() {
+ let expr_a = lit(false).and(col("c2"));
+ let expr_b = col("c2").and(lit(false));
let expected = lit(false);
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_same() -> Result<()> {
- let expr = col("c").and(col("c"));
- let expected = col("c");
+ fn test_simplify_and_same() {
+ let expr = col("c2").and(col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_and_true() -> Result<()> {
- let expr_a = lit(true).and(col("c"));
- let expr_b = col("c").and(lit(true));
- let expected = col("c");
+ fn test_simplify_and_true() {
+ let expr_a = lit(true).and(col("c2"));
+ let expr_b = col("c2").and(lit(true));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_multiply_by_one() -> Result<()> {
- let expr_a = binary_expr(col("c"), Operator::Multiply, lit(1));
- let expr_b = binary_expr(lit(1), Operator::Multiply, col("c"));
- let expected = col("c");
+ fn test_simplify_multiply_by_one() {
+ let expr_a = binary_expr(col("c2"), Operator::Multiply, lit(1));
+ let expr_b = binary_expr(lit(1), Operator::Multiply, col("c2"));
+ let expected = col("c2");
assert_eq!(simplify(&expr_a), expected);
assert_eq!(simplify(&expr_b), expected);
- Ok(())
}
#[test]
- fn test_simplify_divide_by_one() -> Result<()> {
- let expr = binary_expr(col("c"), Operator::Divide, lit(1));
- let expected = col("c");
+ fn test_simplify_divide_by_one() {
+ let expr = binary_expr(col("c2"), Operator::Divide, lit(1));
+ let expected = col("c2");
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_divide_by_same() -> Result<()> {
- let expr = binary_expr(col("c"), Operator::Divide, col("c"));
+ fn test_simplify_divide_by_same() {
+ let expr = binary_expr(col("c2"), Operator::Divide, col("c2"));
let expected = lit(1);
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_simple_and() -> Result<()> {
+ fn test_simplify_simple_and() {
// (c > 5) AND (c > 5)
- let expr = (col("c").gt(lit(5))).and(col("c").gt(lit(5)));
- let expected = col("c").gt(lit(5));
+ let expr = (col("c2").gt(lit(5))).and(col("c2").gt(lit(5)));
+ let expected = col("c2").gt(lit(5));
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_composed_and() -> Result<()> {
+ fn test_simplify_composed_and() {
// ((c > 5) AND (d < 6)) AND (c > 5)
let expr = binary_expr(
- binary_expr(col("c").gt(lit(5)), Operator::And,
col("d").lt(lit(6))),
+ binary_expr(col("c2").gt(lit(5)), Operator::And,
col("d").lt(lit(6))),
Operator::And,
- col("c").gt(lit(5)),
+ col("c2").gt(lit(5)),
);
let expected =
- binary_expr(col("c").gt(lit(5)), Operator::And,
col("d").lt(lit(6)));
+ binary_expr(col("c2").gt(lit(5)), Operator::And,
col("d").lt(lit(6)));
assert_eq!(simplify(&expr), expected);
- Ok(())
}
#[test]
- fn test_simplify_negated_and() -> Result<()> {
+ fn test_simplify_negated_and() {
Review comment:
Tracking in https://github.com/apache/arrow-datafusion/issues/1406
--
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]
