alamb commented on a change in pull request #436: URL: https://github.com/apache/arrow-datafusion/pull/436#discussion_r644357298
########## File path: datafusion/src/optimizer/simplify_expressions.rs ########## @@ -0,0 +1,538 @@ +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Remove duplicate filters optimizer rule Review comment: ```suggestion //! Simplify expressions optimizer rule ``` ########## File path: datafusion/src/optimizer/simplify_expressions.rs ########## @@ -0,0 +1,538 @@ +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Remove duplicate filters optimizer rule + +use crate::execution::context::ExecutionProps; +use crate::logical_plan::LogicalPlan; +use crate::logical_plan::{lit, Expr}; +use crate::optimizer::optimizer::OptimizerRule; +use crate::optimizer::utils; +use crate::optimizer::utils::optimize_explain; +use crate::scalar::ScalarValue; +use crate::{error::Result, logical_plan::Operator}; + +/// Simplify expressions optimizer. +/// # Introduction +/// It uses boolean algebra laws to simplify or reduce the number of terms in expressions. +/// +/// Filter: b > 2 AND b > 2 +/// is optimized to +/// Filter: b > 2 +pub struct SimplifyExpressions {} + +fn expr_contains(expr: &Expr, needle: &Expr) -> bool { + match expr { + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } => expr_contains(left, needle) || expr_contains(right, needle), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } => expr_contains(left, needle) || expr_contains(right, needle), + _ => expr == needle, + } +} + +fn as_binary_expr(expr: &Expr) -> Option<&Expr> { + match expr { + Expr::BinaryExpr { .. } => Some(expr), + _ => None, + } +} + +fn operator_is_boolean(op: Operator) -> bool { + op == Operator::And || op == Operator::Or +} + +fn is_one(s: &Expr) -> bool { + match s { + Expr::Literal(ScalarValue::Int8(Some(1))) + | Expr::Literal(ScalarValue::Int16(Some(1))) + | Expr::Literal(ScalarValue::Int32(Some(1))) + | Expr::Literal(ScalarValue::Int64(Some(1))) + | Expr::Literal(ScalarValue::UInt8(Some(1))) + | Expr::Literal(ScalarValue::UInt16(Some(1))) + | Expr::Literal(ScalarValue::UInt32(Some(1))) + | Expr::Literal(ScalarValue::UInt64(Some(1))) => true, + Expr::Literal(ScalarValue::Float32(Some(v))) if *v == 1. => true, + Expr::Literal(ScalarValue::Float64(Some(v))) if *v == 1. => true, + _ => false, + } +} + +fn is_true(expr: &Expr) -> bool { + match expr { + Expr::Literal(ScalarValue::Boolean(Some(v))) => *v, + _ => false, + } +} + +fn is_null(expr: &Expr) -> bool { + match expr { + Expr::Literal(v) => v.is_null(), + _ => false, + } +} + +fn is_false(expr: &Expr) -> bool { + match expr { + Expr::Literal(ScalarValue::Boolean(Some(v))) => !(*v), + _ => false, + } +} + +fn simplify(expr: &Expr) -> Expr { + match expr { + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if is_true(left) || is_true(right) => lit(true), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if is_false(left) => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if is_false(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if left == right => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if is_false(left) || is_false(right) => lit(false), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if is_true(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if is_true(left) => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if left == right => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::Multiply, + right, + } if is_one(left) => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::Multiply, + right, + } if is_one(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::Divide, + right, + } if is_one(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::Divide, + right, + } if left == right && is_null(left) => *left.clone(), + Expr::BinaryExpr { + left, + op: Operator::Divide, + right, + } if left == right => lit(1), + Expr::BinaryExpr { left, op, right } + if left == right && operator_is_boolean(*op) => + { + simplify(left) + } + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if expr_contains(left, right) => as_binary_expr(left) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&x.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&*right.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if expr_contains(right, left) => as_binary_expr(right) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&*right.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&*left.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if expr_contains(left, right) => as_binary_expr(left) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&*right.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&x.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if expr_contains(right, left) => as_binary_expr(right) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&*left.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&x.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { left, op, right } => Expr::BinaryExpr { + left: Box::new(simplify(&left)), + op: *op, + right: Box::new(simplify(right)), + }, + _ => expr.clone(), + } +} + +fn optimize(plan: &LogicalPlan) -> Result<LogicalPlan> { + let new_inputs = plan + .inputs() + .iter() + .map(|input| optimize(input)) + .collect::<Result<Vec<_>>>()?; + let expr = plan + .expressions() + .into_iter() + .map(|x| simplify(&x)) + .collect::<Vec<_>>(); + utils::from_plan(&plan, &expr, &new_inputs) +} + +impl OptimizerRule for SimplifyExpressions { + fn name(&self) -> &str { + "simplify_expressions" + } + + fn optimize( + &self, + plan: &LogicalPlan, + execution_props: &ExecutionProps, + ) -> Result<LogicalPlan> { + match plan { + LogicalPlan::Explain { + verbose, + plan, + stringified_plans, + schema, + } => { + let schema = schema.as_ref().to_owned().into(); + optimize_explain( + self, + *verbose, + &*plan, + stringified_plans, + &schema, + execution_props, + ) + } + _ => optimize(plan), + } + } +} + +impl SimplifyExpressions { + #[allow(missing_docs)] + pub fn new() -> Self { + Self {} + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::logical_plan::{and, binary_expr, col, lit, Expr, LogicalPlanBuilder}; + use crate::test::*; + + fn assert_optimized_plan_eq(plan: &LogicalPlan, expected: &str) { + let rule = SimplifyExpressions::new(); + let optimized_plan = rule + .optimize(plan, &ExecutionProps::new()) + .expect("failed to optimize plan"); + let formatted_plan = format!("{:?}", optimized_plan); + assert_eq!(formatted_plan, expected); + } + + #[test] + fn test_simplify_or_true() -> Result<()> { + let expr_a = binary_expr(col("c"), Operator::Or, lit(true)); Review comment: FWIW you can also create `or` operators using `Expr::or` For example: ```suggestion let expr_a = col("c").or(lit(true))); ``` ########## File path: datafusion/src/optimizer/simplify_expressions.rs ########## @@ -0,0 +1,538 @@ +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Remove duplicate filters optimizer rule + +use crate::execution::context::ExecutionProps; +use crate::logical_plan::LogicalPlan; +use crate::logical_plan::{lit, Expr}; +use crate::optimizer::optimizer::OptimizerRule; +use crate::optimizer::utils; +use crate::optimizer::utils::optimize_explain; +use crate::scalar::ScalarValue; +use crate::{error::Result, logical_plan::Operator}; + +/// Simplify expressions optimizer. +/// # Introduction +/// It uses boolean algebra laws to simplify or reduce the number of terms in expressions. +/// +/// Filter: b > 2 AND b > 2 +/// is optimized to +/// Filter: b > 2 +pub struct SimplifyExpressions {} Review comment: I can't help but notice the similarity between this pass and `ConstantFolding`: https://github.com/apache/arrow-datafusion/blob/master/datafusion/src/optimizer/constant_folding.rs#L130 Perhaps as a follow on PR, we could combine the code into a single pass ########## File path: datafusion/src/optimizer/simplify_expressions.rs ########## @@ -0,0 +1,538 @@ +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Remove duplicate filters optimizer rule + +use crate::execution::context::ExecutionProps; +use crate::logical_plan::LogicalPlan; +use crate::logical_plan::{lit, Expr}; +use crate::optimizer::optimizer::OptimizerRule; +use crate::optimizer::utils; +use crate::optimizer::utils::optimize_explain; +use crate::scalar::ScalarValue; +use crate::{error::Result, logical_plan::Operator}; + +/// Simplify expressions optimizer. +/// # Introduction +/// It uses boolean algebra laws to simplify or reduce the number of terms in expressions. +/// +/// Filter: b > 2 AND b > 2 +/// is optimized to +/// Filter: b > 2 +pub struct SimplifyExpressions {} + +fn expr_contains(expr: &Expr, needle: &Expr) -> bool { + match expr { + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } => expr_contains(left, needle) || expr_contains(right, needle), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } => expr_contains(left, needle) || expr_contains(right, needle), + _ => expr == needle, + } +} + +fn as_binary_expr(expr: &Expr) -> Option<&Expr> { + match expr { + Expr::BinaryExpr { .. } => Some(expr), + _ => None, + } +} + +fn operator_is_boolean(op: Operator) -> bool { + op == Operator::And || op == Operator::Or +} + +fn is_one(s: &Expr) -> bool { + match s { + Expr::Literal(ScalarValue::Int8(Some(1))) + | Expr::Literal(ScalarValue::Int16(Some(1))) + | Expr::Literal(ScalarValue::Int32(Some(1))) + | Expr::Literal(ScalarValue::Int64(Some(1))) + | Expr::Literal(ScalarValue::UInt8(Some(1))) + | Expr::Literal(ScalarValue::UInt16(Some(1))) + | Expr::Literal(ScalarValue::UInt32(Some(1))) + | Expr::Literal(ScalarValue::UInt64(Some(1))) => true, + Expr::Literal(ScalarValue::Float32(Some(v))) if *v == 1. => true, + Expr::Literal(ScalarValue::Float64(Some(v))) if *v == 1. => true, + _ => false, + } +} + +fn is_true(expr: &Expr) -> bool { + match expr { + Expr::Literal(ScalarValue::Boolean(Some(v))) => *v, + _ => false, + } +} + +fn is_null(expr: &Expr) -> bool { + match expr { + Expr::Literal(v) => v.is_null(), + _ => false, + } +} + +fn is_false(expr: &Expr) -> bool { + match expr { + Expr::Literal(ScalarValue::Boolean(Some(v))) => !(*v), + _ => false, + } +} + +fn simplify(expr: &Expr) -> Expr { + match expr { + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if is_true(left) || is_true(right) => lit(true), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if is_false(left) => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if is_false(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if left == right => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if is_false(left) || is_false(right) => lit(false), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if is_true(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if is_true(left) => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if left == right => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::Multiply, + right, + } if is_one(left) => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::Multiply, + right, + } if is_one(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::Divide, + right, + } if is_one(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::Divide, + right, + } if left == right && is_null(left) => *left.clone(), + Expr::BinaryExpr { + left, + op: Operator::Divide, + right, + } if left == right => lit(1), + Expr::BinaryExpr { left, op, right } + if left == right && operator_is_boolean(*op) => + { + simplify(left) + } + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if expr_contains(left, right) => as_binary_expr(left) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&x.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&*right.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if expr_contains(right, left) => as_binary_expr(right) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&*right.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&*left.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if expr_contains(left, right) => as_binary_expr(left) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&*right.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&x.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if expr_contains(right, left) => as_binary_expr(right) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&*left.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&x.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { left, op, right } => Expr::BinaryExpr { + left: Box::new(simplify(&left)), + op: *op, + right: Box::new(simplify(right)), + }, + _ => expr.clone(), + } +} + +fn optimize(plan: &LogicalPlan) -> Result<LogicalPlan> { + let new_inputs = plan + .inputs() + .iter() + .map(|input| optimize(input)) + .collect::<Result<Vec<_>>>()?; + let expr = plan + .expressions() + .into_iter() + .map(|x| simplify(&x)) + .collect::<Vec<_>>(); + utils::from_plan(&plan, &expr, &new_inputs) +} + +impl OptimizerRule for SimplifyExpressions { + fn name(&self) -> &str { + "simplify_expressions" + } + + fn optimize( + &self, + plan: &LogicalPlan, + execution_props: &ExecutionProps, + ) -> Result<LogicalPlan> { + match plan { + LogicalPlan::Explain { + verbose, + plan, + stringified_plans, + schema, + } => { + let schema = schema.as_ref().to_owned().into(); + optimize_explain( + self, + *verbose, + &*plan, + stringified_plans, + &schema, + execution_props, + ) + } + _ => optimize(plan), + } + } +} + +impl SimplifyExpressions { + #[allow(missing_docs)] + pub fn new() -> Self { + Self {} + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::logical_plan::{and, binary_expr, col, lit, Expr, LogicalPlanBuilder}; + use crate::test::*; + + fn assert_optimized_plan_eq(plan: &LogicalPlan, expected: &str) { + let rule = SimplifyExpressions::new(); + let optimized_plan = rule + .optimize(plan, &ExecutionProps::new()) + .expect("failed to optimize plan"); + let formatted_plan = format!("{:?}", optimized_plan); + assert_eq!(formatted_plan, expected); + } + + #[test] + fn test_simplify_or_true() -> Result<()> { + let expr_a = binary_expr(col("c"), Operator::Or, lit(true)); + let expr_b = binary_expr(lit(true), Operator::Or, col("c")); + 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 = binary_expr(lit(false), Operator::Or, col("c")); + let expr_b = binary_expr(col("c"), Operator::Or, lit(false)); + let expected = col("c"); + + assert_eq!(simplify(&expr_a), expected); + assert_eq!(simplify(&expr_b), expected); + Ok(()) + } + + #[test] + fn test_simplify_or_same() -> Result<()> { + let expr = binary_expr(col("c"), Operator::Or, col("c")); + let expected = col("c"); + + assert_eq!(simplify(&expr), expected); + Ok(()) + } + + #[test] + fn test_simplify_and_false() -> Result<()> { + let expr_a = binary_expr(lit(false), Operator::And, col("c")); + let expr_b = binary_expr(col("c"), Operator::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 = binary_expr(col("c"), Operator::And, col("c")); + let expected = col("c"); + + assert_eq!(simplify(&expr), expected); + Ok(()) + } + + #[test] + fn test_simplify_and_true() -> Result<()> { + let expr_a = binary_expr(lit(true), Operator::And, col("c")); + let expr_b = binary_expr(col("c"), Operator::And, lit(true)); + let expected = col("c"); + + 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"); + + 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"); + + assert_eq!(simplify(&expr), expected); + Ok(()) + } + + #[test] + fn test_simplify_divide_by_same() -> Result<()> { + let expr = binary_expr(col("c"), Operator::Divide, col("c")); + let expected = lit(1); + + assert_eq!(simplify(&expr), expected); + Ok(()) + } + + #[test] + fn test_simplify_simple_and() -> Result<()> { + // (c > 5) AND (c > 5) + let expr = binary_expr(col("c").gt(lit(5)), Operator::And, col("c").gt(lit(5))); + let expected = col("c").gt(lit(5)); + + assert_eq!(simplify(&expr), expected); + Ok(()) + } + + #[test] + fn test_simplify_composed_and() -> Result<()> { + // ((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))), + Operator::And, + col("c").gt(lit(5)), + ); + let expected = + binary_expr(col("c").gt(lit(5)), Operator::And, col("d").lt(lit(6))); + + assert_eq!(simplify(&expr), expected); + Ok(()) + } + + #[test] + fn test_simplify_negated_and() -> Result<()> { + // (c > 5) AND !(c > 5) -- can't remove + let expr = binary_expr( + col("c").gt(lit(5)), + Operator::And, + Expr::not(col("c").gt(lit(5))), + ); + let expected = expr.clone(); + + assert_eq!(simplify(&expr), expected); + Ok(()) + } + + #[test] + fn test_simplify_or_and() -> Result<()> { + // (c > 5) OR ((d < 6) AND (c > 5) -- can't remove Review comment: ```suggestion // (c > 5) OR ((d < 6) AND (c > 5) -- can remove ``` ########## File path: datafusion/src/optimizer/simplify_expressions.rs ########## @@ -0,0 +1,538 @@ +// regarding copyright ownership. The ASF licenses this file Review comment: ```suggestion // Licensed to the Apache Software Foundation (ASF) under one // or more contributor license agreements. See the NOTICE file // distributed with this work for additional information // regarding copyright ownership. The ASF licenses this file ``` The first few lines seem to be missing from the RAT ########## File path: datafusion/src/optimizer/simplify_expressions.rs ########## @@ -0,0 +1,538 @@ +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Remove duplicate filters optimizer rule + +use crate::execution::context::ExecutionProps; +use crate::logical_plan::LogicalPlan; +use crate::logical_plan::{lit, Expr}; +use crate::optimizer::optimizer::OptimizerRule; +use crate::optimizer::utils; +use crate::optimizer::utils::optimize_explain; +use crate::scalar::ScalarValue; +use crate::{error::Result, logical_plan::Operator}; + +/// Simplify expressions optimizer. +/// # Introduction +/// It uses boolean algebra laws to simplify or reduce the number of terms in expressions. +/// +/// Filter: b > 2 AND b > 2 +/// is optimized to +/// Filter: b > 2 +pub struct SimplifyExpressions {} + +fn expr_contains(expr: &Expr, needle: &Expr) -> bool { + match expr { + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } => expr_contains(left, needle) || expr_contains(right, needle), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } => expr_contains(left, needle) || expr_contains(right, needle), + _ => expr == needle, + } +} + +fn as_binary_expr(expr: &Expr) -> Option<&Expr> { + match expr { + Expr::BinaryExpr { .. } => Some(expr), + _ => None, + } +} + +fn operator_is_boolean(op: Operator) -> bool { + op == Operator::And || op == Operator::Or +} + +fn is_one(s: &Expr) -> bool { + match s { + Expr::Literal(ScalarValue::Int8(Some(1))) + | Expr::Literal(ScalarValue::Int16(Some(1))) + | Expr::Literal(ScalarValue::Int32(Some(1))) + | Expr::Literal(ScalarValue::Int64(Some(1))) + | Expr::Literal(ScalarValue::UInt8(Some(1))) + | Expr::Literal(ScalarValue::UInt16(Some(1))) + | Expr::Literal(ScalarValue::UInt32(Some(1))) + | Expr::Literal(ScalarValue::UInt64(Some(1))) => true, + Expr::Literal(ScalarValue::Float32(Some(v))) if *v == 1. => true, + Expr::Literal(ScalarValue::Float64(Some(v))) if *v == 1. => true, + _ => false, + } +} + +fn is_true(expr: &Expr) -> bool { + match expr { + Expr::Literal(ScalarValue::Boolean(Some(v))) => *v, + _ => false, + } +} + +fn is_null(expr: &Expr) -> bool { + match expr { + Expr::Literal(v) => v.is_null(), + _ => false, + } +} + +fn is_false(expr: &Expr) -> bool { + match expr { + Expr::Literal(ScalarValue::Boolean(Some(v))) => !(*v), + _ => false, + } +} + +fn simplify(expr: &Expr) -> Expr { + match expr { + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if is_true(left) || is_true(right) => lit(true), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if is_false(left) => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if is_false(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if left == right => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if is_false(left) || is_false(right) => lit(false), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if is_true(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if is_true(left) => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if left == right => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::Multiply, + right, + } if is_one(left) => simplify(right), + Expr::BinaryExpr { + left, + op: Operator::Multiply, + right, + } if is_one(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::Divide, + right, + } if is_one(right) => simplify(left), + Expr::BinaryExpr { + left, + op: Operator::Divide, + right, + } if left == right && is_null(left) => *left.clone(), + Expr::BinaryExpr { + left, + op: Operator::Divide, + right, + } if left == right => lit(1), + Expr::BinaryExpr { left, op, right } + if left == right && operator_is_boolean(*op) => + { + simplify(left) + } + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if expr_contains(left, right) => as_binary_expr(left) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&x.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&*right.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { + left, + op: Operator::Or, + right, + } if expr_contains(right, left) => as_binary_expr(right) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&*right.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&*left.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if expr_contains(left, right) => as_binary_expr(left) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&*right.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&x.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { + left, + op: Operator::And, + right, + } if expr_contains(right, left) => as_binary_expr(right) + .map(|x| match x { + Expr::BinaryExpr { + left: _, + op: Operator::Or, + right: _, + } => simplify(&*left.clone()), + Expr::BinaryExpr { + left: _, + op: Operator::And, + right: _, + } => simplify(&x.clone()), + _ => expr.clone(), + }) + .unwrap_or_else(|| expr.clone()), + Expr::BinaryExpr { left, op, right } => Expr::BinaryExpr { + left: Box::new(simplify(&left)), + op: *op, + right: Box::new(simplify(right)), + }, + _ => expr.clone(), + } +} + +fn optimize(plan: &LogicalPlan) -> Result<LogicalPlan> { + let new_inputs = plan + .inputs() + .iter() + .map(|input| optimize(input)) + .collect::<Result<Vec<_>>>()?; + let expr = plan + .expressions() + .into_iter() + .map(|x| simplify(&x)) + .collect::<Vec<_>>(); + utils::from_plan(&plan, &expr, &new_inputs) +} + +impl OptimizerRule for SimplifyExpressions { + fn name(&self) -> &str { + "simplify_expressions" + } + + fn optimize( + &self, + plan: &LogicalPlan, + execution_props: &ExecutionProps, + ) -> Result<LogicalPlan> { + match plan { + LogicalPlan::Explain { + verbose, + plan, + stringified_plans, + schema, + } => { + let schema = schema.as_ref().to_owned().into(); + optimize_explain( + self, + *verbose, + &*plan, + stringified_plans, + &schema, + execution_props, + ) + } + _ => optimize(plan), + } + } +} + +impl SimplifyExpressions { + #[allow(missing_docs)] + pub fn new() -> Self { + Self {} + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::logical_plan::{and, binary_expr, col, lit, Expr, LogicalPlanBuilder}; + use crate::test::*; + + fn assert_optimized_plan_eq(plan: &LogicalPlan, expected: &str) { + let rule = SimplifyExpressions::new(); + let optimized_plan = rule + .optimize(plan, &ExecutionProps::new()) + .expect("failed to optimize plan"); + let formatted_plan = format!("{:?}", optimized_plan); + assert_eq!(formatted_plan, expected); + } + + #[test] + fn test_simplify_or_true() -> Result<()> { + let expr_a = binary_expr(col("c"), Operator::Or, lit(true)); + let expr_b = binary_expr(lit(true), Operator::Or, col("c")); + 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 = binary_expr(lit(false), Operator::Or, col("c")); + let expr_b = binary_expr(col("c"), Operator::Or, lit(false)); + let expected = col("c"); + + assert_eq!(simplify(&expr_a), expected); + assert_eq!(simplify(&expr_b), expected); + Ok(()) + } + + #[test] + fn test_simplify_or_same() -> Result<()> { + let expr = binary_expr(col("c"), Operator::Or, col("c")); + let expected = col("c"); + + assert_eq!(simplify(&expr), expected); + Ok(()) + } + + #[test] + fn test_simplify_and_false() -> Result<()> { + let expr_a = binary_expr(lit(false), Operator::And, col("c")); + let expr_b = binary_expr(col("c"), Operator::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 = binary_expr(col("c"), Operator::And, col("c")); + let expected = col("c"); + + assert_eq!(simplify(&expr), expected); + Ok(()) + } + + #[test] + fn test_simplify_and_true() -> Result<()> { + let expr_a = binary_expr(lit(true), Operator::And, col("c")); + let expr_b = binary_expr(col("c"), Operator::And, lit(true)); + let expected = col("c"); + + 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"); + + 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"); + + assert_eq!(simplify(&expr), expected); + Ok(()) + } + + #[test] + fn test_simplify_divide_by_same() -> Result<()> { + let expr = binary_expr(col("c"), Operator::Divide, col("c")); Review comment: I wonder if we care that in some cases (where c has a `0`) this simplification will avoid a runtime error when it would have generated one without the optimization pass I personally think it is ok, but wanted to mention it -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
