alamb commented on code in PR #5423:
URL: https://github.com/apache/arrow-datafusion/pull/5423#discussion_r1120767458
##########
datafusion/expr/src/expr.rs:
##########
@@ -639,6 +639,31 @@ impl Expr {
binary_expr(self, Operator::Or, other)
}
+ /// Return `self & other`
Review Comment:
👍
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -700,6 +701,288 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a,
S> {
return
Err(DataFusionError::ArrowError(ArrowError::DivideByZero));
}
+ //
+ // Rules for BitwiseAnd
+ //
+
+ // A & null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null & A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A & 0 -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_zero(&right) => *right,
Review Comment:
I think this rule also has to check that `A` is non nullable as `null & 0`
is `null` (not `0`, which is what this code does)
```sql
postgres=# select NULL & 0 ;
?column?
----------
(1 row)
```
The same comment applies to `0 & A` as well
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -1346,6 +1629,361 @@ mod tests {
assert_eq!(simplify(expr), expected);
}
+ #[test]
+ fn test_simplify_bitwise_xor_by_null() {
+ let null = Expr::Literal(ScalarValue::Null);
+ // A ^ null --> null
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseXor,
null.clone());
+ assert_eq!(simplify(expr), null);
+ }
+ // null ^ A --> null
+ {
+ let expr = binary_expr(null.clone(), Operator::BitwiseXor,
col("c2"));
+ assert_eq!(simplify(expr), null);
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_shift_right_by_null() {
+ let null = Expr::Literal(ScalarValue::Null);
+ // A >> null --> null
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseShiftRight,
null.clone());
+ assert_eq!(simplify(expr), null);
+ }
+ // null >> A --> null
+ {
+ let expr = binary_expr(null.clone(), Operator::BitwiseShiftRight,
col("c2"));
+ assert_eq!(simplify(expr), null);
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_shift_left_by_null() {
+ let null = Expr::Literal(ScalarValue::Null);
+ // A << null --> null
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseShiftLeft,
null.clone());
+ assert_eq!(simplify(expr), null);
+ }
+ // null << A --> null
+ {
+ let expr = binary_expr(null.clone(), Operator::BitwiseShiftLeft,
col("c2"));
+ assert_eq!(simplify(expr), null);
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_and_by_zero() {
+ // A & 0 --> 0
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseAnd, lit(0));
+ assert_eq!(simplify(expr), lit(0));
+ }
+ // 0 & A --> 0
+ {
+ let expr = binary_expr(lit(0), Operator::BitwiseAnd, col("c2"));
+ assert_eq!(simplify(expr), lit(0));
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_or_by_zero() {
+ // A | 0 --> A
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseOr, lit(0));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ // 0 | A --> A
+ {
+ let expr = binary_expr(lit(0), Operator::BitwiseOr, col("c2"));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_xor_by_zero() {
+ // A ^ 0 --> A
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseXor, lit(0));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ // 0 ^ A --> A
+ {
+ let expr = binary_expr(lit(0), Operator::BitwiseXor, col("c2"));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_bitwise_shift_right_by_zero() {
+ // A >> 0 --> A
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseShiftRight,
lit(0));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_bitwise_shift_left_by_zero() {
+ // A << 0 --> A
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseShiftLeft,
lit(0));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_and_by_null() {
+ let null = Expr::Literal(ScalarValue::Null);
+ // A & null --> null
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseAnd,
null.clone());
+ assert_eq!(simplify(expr), null);
+ }
+ // null & A --> null
+ {
+ let expr = binary_expr(null.clone(), Operator::BitwiseAnd,
col("c2"));
+ assert_eq!(simplify(expr), null);
+ }
+ }
+
+ #[test]
+ fn test_simplify_composed_bitwise_and() {
+ // ((c > 5) & (c1 < 6)) & (c > 5) --> ((c > 5) & c1)
+
+ let expr = binary_expr(
+ binary_expr(
+ col("c2").gt(lit(5)),
+ Operator::BitwiseAnd,
+ col("c1").lt(lit(6)),
+ ),
+ Operator::BitwiseAnd,
+ col("c2").gt(lit(5)),
+ );
+ let expected = binary_expr(
+ col("c2").gt(lit(5)),
+ Operator::BitwiseAnd,
+ col("c1").lt(lit(6)),
+ );
+
+ assert_eq!(simplify(expr), expected);
+ }
+
+ #[test]
+ fn test_simplify_composed_bitwise_or() {
+ // ((c > 5) | (c1 < 6)) | (c > 5) --> ((c > 5) | c1)
Review Comment:
```suggestion
// ((c2 > 5) | (c1 < 6)) | (c2 > 5) --> ((c2 > 5) | c1)
```
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -700,6 +701,288 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a,
S> {
return
Err(DataFusionError::ArrowError(ArrowError::DivideByZero));
}
+ //
+ // Rules for BitwiseAnd
+ //
+
+ // A & null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null & A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A & 0 -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_zero(&right) => *right,
+
+ // 0 & A -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_zero(&left) => *left,
+
+ // !A & A -> 0 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if is_negative_of(&left, &right) && !info.nullable(&right)? => {
+ Expr::Literal(ScalarValue::Int32(Some(0)))
+ }
+
+ // A & !A -> 0 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if is_negative_of(&right, &left) && !info.nullable(&left)? => {
+ Expr::Literal(ScalarValue::Int32(Some(0)))
+ }
+
+ // (..A..) & A --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if expr_contains(&left, &right, BitwiseAnd) => *left,
+
+ // A & (..A..) --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if expr_contains(&right, &left, BitwiseAnd) => *right,
+
+ // A & (A | B) --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if !info.nullable(&right)? && is_op_with(BitwiseOr, &right,
&left) => {
+ *left
+ }
+
+ // (A | B) & A --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if !info.nullable(&left)? && is_op_with(BitwiseOr, &left,
&right) => {
+ *right
+ }
+
+ //
+ // Rules for BitwiseOr
+ //
+
+ // A | null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseOr,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null | A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A | 0 -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_zero(&right) => *left,
+
+ // 0 | A -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_zero(&left) => *right,
+
+ // !A | A -> -1 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_negative_of(&left, &right) && !info.nullable(&right)? => {
+ Expr::Literal(ScalarValue::Int32(Some(-1)))
+ }
+
+ // A | !A -> -1 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_negative_of(&right, &left) && !info.nullable(&left)? => {
+ Expr::Literal(ScalarValue::Int32(Some(-1)))
+ }
+
+ // (..A..) | A --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if expr_contains(&left, &right, BitwiseOr) => *left,
+
+ // A | (..A..) --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if expr_contains(&left, &right, BitwiseOr) => *left,
+
+ // A | (A & B) --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if !info.nullable(&right)? && is_op_with(BitwiseAnd, &right,
&left) => {
+ *left
+ }
+
+ // (A & B) | A --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if !info.nullable(&left)? && is_op_with(BitwiseAnd, &left,
&right) => {
+ *right
+ }
+
+ //
+ // Rules for BitwiseXor
+ //
+
+ // A ^ null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseXor,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null ^ A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseXor,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A ^ 0 -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseXor,
+ right,
+ }) if is_zero(&right) => *left,
+
+ // 0 ^ A -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseXor,
+ right,
+ }) if is_zero(&left) => *right,
+
+ // !A ^ A -> -1 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseXor,
+ right,
+ }) if is_negative_of(&left, &right) && !info.nullable(&right)? => {
+ Expr::Literal(ScalarValue::Int32(Some(-1)))
+ }
+
+ // A ^ !A -> -1 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseXor,
+ right,
+ }) if is_negative_of(&right, &left) && !info.nullable(&left)? => {
+ Expr::Literal(ScalarValue::Int32(Some(-1)))
Review Comment:
needs the right `ScalarValue` variant
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -700,6 +701,288 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a,
S> {
return
Err(DataFusionError::ArrowError(ArrowError::DivideByZero));
}
+ //
+ // Rules for BitwiseAnd
+ //
+
+ // A & null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null & A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A & 0 -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_zero(&right) => *right,
+
+ // 0 & A -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_zero(&left) => *left,
+
+ // !A & A -> 0 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if is_negative_of(&left, &right) && !info.nullable(&right)? => {
+ Expr::Literal(ScalarValue::Int32(Some(0)))
+ }
+
+ // A & !A -> 0 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if is_negative_of(&right, &left) && !info.nullable(&left)? => {
+ Expr::Literal(ScalarValue::Int32(Some(0)))
+ }
+
+ // (..A..) & A --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if expr_contains(&left, &right, BitwiseAnd) => *left,
+
+ // A & (..A..) --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if expr_contains(&right, &left, BitwiseAnd) => *right,
+
+ // A & (A | B) --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if !info.nullable(&right)? && is_op_with(BitwiseOr, &right,
&left) => {
+ *left
+ }
+
+ // (A | B) & A --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if !info.nullable(&left)? && is_op_with(BitwiseOr, &left,
&right) => {
+ *right
+ }
+
+ //
+ // Rules for BitwiseOr
+ //
+
+ // A | null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseOr,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null | A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A | 0 -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_zero(&right) => *left,
+
+ // 0 | A -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_zero(&left) => *right,
+
+ // !A | A -> -1 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_negative_of(&left, &right) && !info.nullable(&right)? => {
+ Expr::Literal(ScalarValue::Int32(Some(-1)))
+ }
+
+ // A | !A -> -1 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_negative_of(&right, &left) && !info.nullable(&left)? => {
+ Expr::Literal(ScalarValue::Int32(Some(-1)))
+ }
+
+ // (..A..) | A --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if expr_contains(&left, &right, BitwiseOr) => *left,
+
+ // A | (..A..) --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if expr_contains(&left, &right, BitwiseOr) => *left,
+
+ // A | (A & B) --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if !info.nullable(&right)? && is_op_with(BitwiseAnd, &right,
&left) => {
+ *left
+ }
+
+ // (A & B) | A --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if !info.nullable(&left)? && is_op_with(BitwiseAnd, &left,
&right) => {
+ *right
+ }
+
+ //
+ // Rules for BitwiseXor
+ //
+
+ // A ^ null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseXor,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null ^ A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseXor,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A ^ 0 -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseXor,
+ right,
+ }) if is_zero(&right) => *left,
+
+ // 0 ^ A -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseXor,
+ right,
+ }) if is_zero(&left) => *right,
+
+ // !A ^ A -> -1 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseXor,
+ right,
+ }) if is_negative_of(&left, &right) && !info.nullable(&right)? => {
+ Expr::Literal(ScalarValue::Int32(Some(-1)))
Review Comment:
needs the right `ScalarValue` variant
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -700,6 +701,288 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a,
S> {
return
Err(DataFusionError::ArrowError(ArrowError::DivideByZero));
}
+ //
+ // Rules for BitwiseAnd
+ //
+
+ // A & null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null & A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A & 0 -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_zero(&right) => *right,
+
+ // 0 & A -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_zero(&left) => *left,
+
+ // !A & A -> 0 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if is_negative_of(&left, &right) && !info.nullable(&right)? => {
+ Expr::Literal(ScalarValue::Int32(Some(0)))
+ }
+
+ // A & !A -> 0 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if is_negative_of(&right, &left) && !info.nullable(&left)? => {
+ Expr::Literal(ScalarValue::Int32(Some(0)))
+ }
+
+ // (..A..) & A --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if expr_contains(&left, &right, BitwiseAnd) => *left,
+
+ // A & (..A..) --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if expr_contains(&right, &left, BitwiseAnd) => *right,
+
+ // A & (A | B) --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if !info.nullable(&right)? && is_op_with(BitwiseOr, &right,
&left) => {
+ *left
+ }
+
+ // (A | B) & A --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if !info.nullable(&left)? && is_op_with(BitwiseOr, &left,
&right) => {
+ *right
+ }
+
+ //
+ // Rules for BitwiseOr
+ //
+
+ // A | null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseOr,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null | A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A | 0 -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_zero(&right) => *left,
+
+ // 0 | A -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_zero(&left) => *right,
+
+ // !A | A -> -1 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_negative_of(&left, &right) && !info.nullable(&right)? => {
+ Expr::Literal(ScalarValue::Int32(Some(-1)))
Review Comment:
Likewise I think can't always use `ScalarValue::Int32` -- it needs to use
the appropriate variant of `ScalarValue` for the type of left/right
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -700,6 +701,288 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a,
S> {
return
Err(DataFusionError::ArrowError(ArrowError::DivideByZero));
}
+ //
+ // Rules for BitwiseAnd
+ //
+
+ // A & null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null & A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A & 0 -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_zero(&right) => *right,
+
+ // 0 & A -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_zero(&left) => *left,
+
+ // !A & A -> 0 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if is_negative_of(&left, &right) && !info.nullable(&right)? => {
+ Expr::Literal(ScalarValue::Int32(Some(0)))
Review Comment:
It seems like this should not always be `ScalarValue::Int32` -- if `left`
and `right` are `DataType::Int64` for example, I think this should be
`ScalarValue::Int64(Some(0))`. This applies to the other rules as well.
Perhaps we could add a method like `ScalarValue::new_zero(data_type:
DataType)` to create such a constant
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -700,6 +701,288 @@ impl<'a, S: SimplifyInfo> ExprRewriter for Simplifier<'a,
S> {
return
Err(DataFusionError::ArrowError(ArrowError::DivideByZero));
}
+ //
+ // Rules for BitwiseAnd
+ //
+
+ // A & null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null & A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A & 0 -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseAnd,
+ right,
+ }) if is_zero(&right) => *right,
+
+ // 0 & A -> 0
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right: _,
+ }) if is_zero(&left) => *left,
+
+ // !A & A -> 0 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if is_negative_of(&left, &right) && !info.nullable(&right)? => {
+ Expr::Literal(ScalarValue::Int32(Some(0)))
+ }
+
+ // A & !A -> 0 (if A not nullable)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if is_negative_of(&right, &left) && !info.nullable(&left)? => {
+ Expr::Literal(ScalarValue::Int32(Some(0)))
+ }
+
+ // (..A..) & A --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if expr_contains(&left, &right, BitwiseAnd) => *left,
+
+ // A & (..A..) --> (..A..)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if expr_contains(&right, &left, BitwiseAnd) => *right,
+
+ // A & (A | B) --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if !info.nullable(&right)? && is_op_with(BitwiseOr, &right,
&left) => {
+ *left
+ }
+
+ // (A | B) & A --> A (if B not null)
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseAnd,
+ right,
+ }) if !info.nullable(&left)? && is_op_with(BitwiseOr, &left,
&right) => {
+ *right
+ }
+
+ //
+ // Rules for BitwiseOr
+ //
+
+ // A | null -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left: _,
+ op: BitwiseOr,
+ right,
+ }) if is_null(&right) => *right,
+
+ // null | A -> null
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right: _,
+ }) if is_null(&left) => *left,
+
+ // A | 0 -> A
+ Expr::BinaryExpr(BinaryExpr {
+ left,
+ op: BitwiseOr,
+ right,
+ }) if is_zero(&right) => *left,
Review Comment:
same thing here -- I think it only applies if `right` is not `NULL`
##########
datafusion/optimizer/src/simplify_expressions/utils.rs:
##########
@@ -77,6 +78,80 @@ pub fn expr_contains(expr: &Expr, needle: &Expr, search_op:
Operator) -> bool {
}
}
+/// This enum is needed for the function "delete_expressions_in_complex_expr"
for
Review Comment:
```suggestion
/// This enum is needed for the function "delete_xor_in_complex_expr" for
```
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -1346,6 +1629,361 @@ mod tests {
assert_eq!(simplify(expr), expected);
}
+ #[test]
+ fn test_simplify_bitwise_xor_by_null() {
+ let null = Expr::Literal(ScalarValue::Null);
+ // A ^ null --> null
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseXor,
null.clone());
+ assert_eq!(simplify(expr), null);
+ }
+ // null ^ A --> null
+ {
+ let expr = binary_expr(null.clone(), Operator::BitwiseXor,
col("c2"));
+ assert_eq!(simplify(expr), null);
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_shift_right_by_null() {
+ let null = Expr::Literal(ScalarValue::Null);
+ // A >> null --> null
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseShiftRight,
null.clone());
+ assert_eq!(simplify(expr), null);
+ }
+ // null >> A --> null
+ {
+ let expr = binary_expr(null.clone(), Operator::BitwiseShiftRight,
col("c2"));
+ assert_eq!(simplify(expr), null);
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_shift_left_by_null() {
+ let null = Expr::Literal(ScalarValue::Null);
+ // A << null --> null
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseShiftLeft,
null.clone());
+ assert_eq!(simplify(expr), null);
+ }
+ // null << A --> null
+ {
+ let expr = binary_expr(null.clone(), Operator::BitwiseShiftLeft,
col("c2"));
+ assert_eq!(simplify(expr), null);
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_and_by_zero() {
+ // A & 0 --> 0
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseAnd, lit(0));
+ assert_eq!(simplify(expr), lit(0));
+ }
+ // 0 & A --> 0
+ {
+ let expr = binary_expr(lit(0), Operator::BitwiseAnd, col("c2"));
+ assert_eq!(simplify(expr), lit(0));
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_or_by_zero() {
+ // A | 0 --> A
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseOr, lit(0));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ // 0 | A --> A
+ {
+ let expr = binary_expr(lit(0), Operator::BitwiseOr, col("c2"));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_xor_by_zero() {
+ // A ^ 0 --> A
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseXor, lit(0));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ // 0 ^ A --> A
+ {
+ let expr = binary_expr(lit(0), Operator::BitwiseXor, col("c2"));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_bitwise_shift_right_by_zero() {
+ // A >> 0 --> A
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseShiftRight,
lit(0));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_bitwise_shift_left_by_zero() {
+ // A << 0 --> A
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseShiftLeft,
lit(0));
+ assert_eq!(simplify(expr), col("c2"));
+ }
+ }
+
+ #[test]
+ fn test_simplify_bitwise_and_by_null() {
+ let null = Expr::Literal(ScalarValue::Null);
+ // A & null --> null
+ {
+ let expr = binary_expr(col("c2"), Operator::BitwiseAnd,
null.clone());
+ assert_eq!(simplify(expr), null);
+ }
+ // null & A --> null
+ {
+ let expr = binary_expr(null.clone(), Operator::BitwiseAnd,
col("c2"));
+ assert_eq!(simplify(expr), null);
+ }
+ }
+
+ #[test]
+ fn test_simplify_composed_bitwise_and() {
+ // ((c > 5) & (c1 < 6)) & (c > 5) --> ((c > 5) & c1)
Review Comment:
I think this should refer to c1 and c2 (rather than c and c1)
```
// ((c2 > 5) & (c1 < 6)) & (c2 > 5) --> ((c2 > 5) & c1)
```
It might be valuable to add a test for the other grouping as well:
```
// (c > 5) & ((c1 < 6) & (c > 5)) --> c1 & (c > 5)
```
##########
datafusion/optimizer/src/simplify_expressions/utils.rs:
##########
@@ -154,11 +229,16 @@ pub fn is_op_with(target_op: Operator, haystack: &Expr,
needle: &Expr) -> bool {
matches!(haystack, Expr::BinaryExpr(BinaryExpr { left, op, right }) if op
== &target_op && (needle == left.as_ref() || needle == right.as_ref()))
}
-/// returns true if `not_expr` is !`expr`
+/// returns true if `not_expr` is !`expr` (not)
pub fn is_not_of(not_expr: &Expr, expr: &Expr) -> bool {
matches!(not_expr, Expr::Not(inner) if expr == inner.as_ref())
}
+/// returns true if `not_expr` is !`expr` (bitwise not)
+pub fn is_negative_of(not_expr: &Expr, expr: &Expr) -> bool {
+ matches!(not_expr, Expr::Negative(inner) if expr == inner.as_ref())
Review Comment:
Shouldn't this be checking for `Expr::Not` rather than `Expr::Negative`?
```suggestion
matches!(not_expr, Expr::Not(inner) if expr == inner.as_ref())
```
--
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]
