diff --git a/datafusion/core/src/physical_optimizer/pruning.rs b/datafusion/core/src/physical_optimizer/pruning.rs index e8f2f34abda0..a7ce29bdc7e3 100644 --- a/datafusion/core/src/physical_optimizer/pruning.rs +++ b/datafusion/core/src/physical_optimizer/pruning.rs @@ -987,8 +987,8 @@ impl<'a> PruningExpressionBuilder<'a> { }) } - fn op(&self) -> &Operator { - &self.op + fn op(&self) -> Operator { + self.op } fn scalar_expr(&self) -> &Arc { @@ -1064,7 +1064,7 @@ fn rewrite_expr_to_prunable( scalar_expr: &PhysicalExprRef, schema: DFSchema, ) -> Result<(PhysicalExprRef, Operator, PhysicalExprRef)> { - if !is_compare_op(&op) { + if !is_compare_op(op) { return plan_err!("rewrite_expr_to_prunable only support compare expression"); } @@ -1131,7 +1131,7 @@ fn rewrite_expr_to_prunable( } } -fn is_compare_op(op: &Operator) -> bool { +fn is_compare_op(op: Operator) -> bool { matches!( op, Operator::Eq @@ -1358,13 +1358,11 @@ fn build_predicate_expression( .map(|e| { Arc::new(phys_expr::BinaryExpr::new( in_list.expr().clone(), - eq_op.clone(), + eq_op, e.clone(), )) as _ }) - .reduce(|a, b| { - Arc::new(phys_expr::BinaryExpr::new(a, re_op.clone(), b)) as _ - }) + .reduce(|a, b| Arc::new(phys_expr::BinaryExpr::new(a, re_op, b)) as _) .unwrap(); return build_predicate_expression(&change_expr, schema, required_columns); } else { @@ -1376,7 +1374,7 @@ fn build_predicate_expression( if let Some(bin_expr) = expr_any.downcast_ref::() { ( bin_expr.left().clone(), - bin_expr.op().clone(), + *bin_expr.op(), bin_expr.right().clone(), ) } else { @@ -1388,7 +1386,7 @@ fn build_predicate_expression( let left_expr = build_predicate_expression(&left, schema, required_columns); let right_expr = build_predicate_expression(&right, schema, required_columns); // simplify boolean expression if applicable - let expr = match (&left_expr, &op, &right_expr) { + let expr = match (&left_expr, op, &right_expr) { (left, Operator::And, _) if is_always_true(left) => right_expr, (_, Operator::And, right) if is_always_true(right) => left_expr, (left, Operator::Or, right) @@ -1396,11 +1394,7 @@ fn build_predicate_expression( { unhandled } - _ => Arc::new(phys_expr::BinaryExpr::new( - left_expr, - op.clone(), - right_expr, - )), + _ => Arc::new(phys_expr::BinaryExpr::new(left_expr, op, right_expr)), }; return expr; } diff --git a/datafusion/expr/src/operator.rs b/datafusion/expr/src/operator.rs index 742511822a0f..a10312e23446 100644 --- a/datafusion/expr/src/operator.rs +++ b/datafusion/expr/src/operator.rs @@ -25,7 +25,7 @@ use std::ops; use std::ops::Not; /// Operators applied to expressions -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Hash)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Hash)] pub enum Operator { /// Expressions are equal Eq, diff --git a/datafusion/expr/src/type_coercion/binary.rs b/datafusion/expr/src/type_coercion/binary.rs index 83a7da046844..4f79f3fa2b22 100644 --- a/datafusion/expr/src/type_coercion/binary.rs +++ b/datafusion/expr/src/type_coercion/binary.rs @@ -1152,8 +1152,8 @@ mod tests { ]; for (i, input_type) in input_types.iter().enumerate() { let expect_type = &result_types[i]; - for op in &comparison_op_types { - let (lhs, rhs) = get_input_types(&input_decimal, op, input_type)?; + for op in comparison_op_types { + let (lhs, rhs) = get_input_types(&input_decimal, &op, input_type)?; assert_eq!(expect_type, &lhs); assert_eq!(expect_type, &rhs); } diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs index 34e007207427..45155cbd2c27 100644 --- a/datafusion/expr/src/utils.rs +++ b/datafusion/expr/src/utils.rs @@ -997,7 +997,7 @@ fn split_conjunction_impl<'a>(expr: &'a Expr, mut exprs: Vec<&'a Expr>) -> Vec<& /// assert_eq!(split_conjunction_owned(expr), split); /// ``` pub fn split_conjunction_owned(expr: Expr) -> Vec { - split_binary_owned(expr, &Operator::And) + split_binary_owned(expr, Operator::And) } /// Splits an owned binary operator tree [`Expr`] such as `A B C` => `[A, B, C]` @@ -1020,19 +1020,19 @@ pub fn split_conjunction_owned(expr: Expr) -> Vec { /// ]; /// /// // use split_binary_owned to split them -/// assert_eq!(split_binary_owned(expr, &Operator::Plus), split); +/// assert_eq!(split_binary_owned(expr, Operator::Plus), split); /// ``` -pub fn split_binary_owned(expr: Expr, op: &Operator) -> Vec { +pub fn split_binary_owned(expr: Expr, op: Operator) -> Vec { split_binary_owned_impl(expr, op, vec![]) } fn split_binary_owned_impl( expr: Expr, - operator: &Operator, + operator: Operator, mut exprs: Vec, ) -> Vec { match expr { - Expr::BinaryExpr(BinaryExpr { right, op, left }) if &op == operator => { + Expr::BinaryExpr(BinaryExpr { right, op, left }) if op == operator => { let exprs = split_binary_owned_impl(*left, operator, exprs); split_binary_owned_impl(*right, operator, exprs) } @@ -1049,17 +1049,17 @@ fn split_binary_owned_impl( /// Splits an binary operator tree [`Expr`] such as `A B C` => `[A, B, C]` /// /// See [`split_binary_owned`] for more details and an example. -pub fn split_binary<'a>(expr: &'a Expr, op: &Operator) -> Vec<&'a Expr> { +pub fn split_binary(expr: &Expr, op: Operator) -> Vec<&Expr> { split_binary_impl(expr, op, vec![]) } fn split_binary_impl<'a>( expr: &'a Expr, - operator: &Operator, + operator: Operator, mut exprs: Vec<&'a Expr>, ) -> Vec<&'a Expr> { match expr { - Expr::BinaryExpr(BinaryExpr { right, op, left }) if op == operator => { + Expr::BinaryExpr(BinaryExpr { right, op, left }) if *op == operator => { let exprs = split_binary_impl(left, operator, exprs); split_binary_impl(right, operator, exprs) } @@ -1613,13 +1613,13 @@ mod tests { #[test] fn test_split_binary_owned() { let expr = col("a"); - assert_eq!(split_binary_owned(expr.clone(), &Operator::And), vec![expr]); + assert_eq!(split_binary_owned(expr.clone(), Operator::And), vec![expr]); } #[test] fn test_split_binary_owned_two() { assert_eq!( - split_binary_owned(col("a").eq(lit(5)).and(col("b")), &Operator::And), + split_binary_owned(col("a").eq(lit(5)).and(col("b")), Operator::And), vec![col("a").eq(lit(5)), col("b")] ); } @@ -1629,7 +1629,7 @@ mod tests { let expr = col("a").eq(lit(5)).or(col("b")); assert_eq!( // expr is connected by OR, but pass in AND - split_binary_owned(expr.clone(), &Operator::And), + split_binary_owned(expr.clone(), Operator::And), vec![expr] ); } diff --git a/datafusion/optimizer/src/analyzer/type_coercion.rs b/datafusion/optimizer/src/analyzer/type_coercion.rs index 64d9c508f3f6..6c08b3e998b3 100644 --- a/datafusion/optimizer/src/analyzer/type_coercion.rs +++ b/datafusion/optimizer/src/analyzer/type_coercion.rs @@ -146,7 +146,7 @@ impl<'a> TypeCoercionRewriter<'a> { .map(|(lhs, rhs)| { // coerce the arguments as though they were a single binary equality // expression - let (lhs, rhs) = self.coerce_binary_op(lhs, &Operator::Eq, rhs)?; + let (lhs, rhs) = self.coerce_binary_op(lhs, Operator::Eq, rhs)?; Ok((lhs, rhs)) }) .collect::>>()?; @@ -157,12 +157,12 @@ impl<'a> TypeCoercionRewriter<'a> { fn coerce_binary_op( &self, left: Expr, - op: &Operator, + op: Operator, right: Expr, ) -> Result<(Expr, Expr)> { let (left_type, right_type) = get_input_types( &left.get_type(self.schema)?, - op, + &op, &right.get_type(self.schema)?, )?; Ok(( @@ -279,7 +279,7 @@ impl<'a> TreeNodeRewriter for TypeCoercionRewriter<'a> { )))) } Expr::BinaryExpr(BinaryExpr { left, op, right }) => { - let (left, right) = self.coerce_binary_op(*left, &op, *right)?; + let (left, right) = self.coerce_binary_op(*left, op, *right)?; Ok(Transformed::yes(Expr::BinaryExpr(BinaryExpr::new( Box::new(left), op, diff --git a/datafusion/optimizer/src/simplify_expressions/utils.rs b/datafusion/optimizer/src/simplify_expressions/utils.rs index ed3fd75f3efd..5da727cb5990 100644 --- a/datafusion/optimizer/src/simplify_expressions/utils.rs +++ b/datafusion/optimizer/src/simplify_expressions/utils.rs @@ -69,8 +69,8 @@ pub static POWS_OF_TEN: [i128; 38] = [ /// expressions. Such as: (A AND B) AND C pub fn expr_contains(expr: &Expr, needle: &Expr, search_op: Operator) -> bool { match expr { - Expr::BinaryExpr(BinaryExpr { left, op, right }) if op == &search_op => { - expr_contains(left, needle, search_op.clone()) + Expr::BinaryExpr(BinaryExpr { left, op, right }) if *op == search_op => { + expr_contains(left, needle, search_op) || expr_contains(right, needle, search_op) } _ => expr == needle, @@ -88,7 +88,7 @@ pub fn delete_xor_in_complex_expr(expr: &Expr, needle: &Expr, is_left: bool) -> ) -> Expr { match expr { Expr::BinaryExpr(BinaryExpr { left, op, right }) - if op == &Operator::BitwiseXor => + if *op == Operator::BitwiseXor => { let left_expr = recursive_delete_xor_in_expr(left, needle, xor_counter); let right_expr = recursive_delete_xor_in_expr(right, needle, xor_counter); @@ -102,7 +102,7 @@ pub fn delete_xor_in_complex_expr(expr: &Expr, needle: &Expr, is_left: bool) -> Expr::BinaryExpr(BinaryExpr::new( Box::new(left_expr), - op.clone(), + *op, Box::new(right_expr), )) } diff --git a/datafusion/optimizer/src/utils.rs b/datafusion/optimizer/src/utils.rs index 0549845430a6..05b1744d90c5 100644 --- a/datafusion/optimizer/src/utils.rs +++ b/datafusion/optimizer/src/utils.rs @@ -177,13 +177,13 @@ pub fn split_conjunction_owned(expr: Expr) -> Vec { /// ]; /// /// // use split_binary_owned to split them -/// assert_eq!(split_binary_owned(expr, &Operator::Plus), split); +/// assert_eq!(split_binary_owned(expr, Operator::Plus), split); /// ``` #[deprecated( since = "34.0.0", note = "use `datafusion_expr::utils::split_binary_owned` instead" )] -pub fn split_binary_owned(expr: Expr, op: &Operator) -> Vec { +pub fn split_binary_owned(expr: Expr, op: Operator) -> Vec { expr_utils::split_binary_owned(expr, op) } @@ -194,7 +194,7 @@ pub fn split_binary_owned(expr: Expr, op: &Operator) -> Vec { since = "34.0.0", note = "use `datafusion_expr::utils::split_binary` instead" )] -pub fn split_binary<'a>(expr: &'a Expr, op: &Operator) -> Vec<&'a Expr> { +pub fn split_binary(expr: &Expr, op: Operator) -> Vec<&Expr> { expr_utils::split_binary(expr, op) } diff --git a/datafusion/physical-expr-common/src/datum.rs b/datafusion/physical-expr-common/src/datum.rs index 790e742c4221..d0ba5f113b6f 100644 --- a/datafusion/physical-expr-common/src/datum.rs +++ b/datafusion/physical-expr-common/src/datum.rs @@ -63,7 +63,7 @@ pub fn apply_cmp( /// Applies a binary [`Datum`] comparison kernel `f` to `lhs` and `rhs` for nested type like /// List, FixedSizeList, LargeList, Struct, Union, Map, or a dictionary of a nested type pub fn apply_cmp_for_nested( - op: &Operator, + op: Operator, lhs: &ColumnarValue, rhs: &ColumnarValue, ) -> Result { @@ -88,7 +88,7 @@ pub fn apply_cmp_for_nested( /// Compare on nested type List, Struct, and so on pub fn compare_op_for_nested( - op: &Operator, + op: Operator, lhs: &dyn Datum, rhs: &dyn Datum, ) -> Result { diff --git a/datafusion/physical-expr/src/expressions/binary.rs b/datafusion/physical-expr/src/expressions/binary.rs index f1e40575bc64..c153ead9639f 100644 --- a/datafusion/physical-expr/src/expressions/binary.rs +++ b/datafusion/physical-expr/src/expressions/binary.rs @@ -269,7 +269,7 @@ impl PhysicalExpr for BinaryExpr { if right_data_type != left_data_type { return internal_err!("type mismatch"); } - return apply_cmp_for_nested(&self.op, &lhs, &rhs); + return apply_cmp_for_nested(self.op, &lhs, &rhs); } match self.op { @@ -329,7 +329,7 @@ impl PhysicalExpr for BinaryExpr { ) -> Result> { Ok(Arc::new(BinaryExpr::new( Arc::clone(&children[0]), - self.op.clone(), + self.op, Arc::clone(&children[1]), ))) } diff --git a/datafusion/physical-expr/src/intervals/cp_solver.rs b/datafusion/physical-expr/src/intervals/cp_solver.rs index ef9dd36cfb50..fc4950ae4e7c 100644 --- a/datafusion/physical-expr/src/intervals/cp_solver.rs +++ b/datafusion/physical-expr/src/intervals/cp_solver.rs @@ -222,7 +222,7 @@ pub fn propagate_arithmetic( left_child: &Interval, right_child: &Interval, ) -> Result> { - let inverse_op = get_inverse_op(op)?; + let inverse_op = get_inverse_op(*op)?; match (left_child.data_type(), right_child.data_type()) { // If we have a child whose type is a time interval (i.e. DataType::Interval), // we need special handling since timestamp differencing results in a diff --git a/datafusion/physical-expr/src/intervals/utils.rs b/datafusion/physical-expr/src/intervals/utils.rs index 37527802f84d..b426a656fba9 100644 --- a/datafusion/physical-expr/src/intervals/utils.rs +++ b/datafusion/physical-expr/src/intervals/utils.rs @@ -63,7 +63,7 @@ pub fn check_support(expr: &Arc, schema: &SchemaRef) -> bool { } // This function returns the inverse operator of the given operator. -pub fn get_inverse_op(op: &Operator) -> Result { +pub fn get_inverse_op(op: Operator) -> Result { match op { Operator::Plus => Ok(Operator::Minus), Operator::Minus => Ok(Operator::Plus), diff --git a/datafusion/physical-expr/src/planner.rs b/datafusion/physical-expr/src/planner.rs index dbebf4c18b79..a975f0c6ef83 100644 --- a/datafusion/physical-expr/src/planner.rs +++ b/datafusion/physical-expr/src/planner.rs @@ -195,7 +195,7 @@ pub fn create_physical_expr( // // There should be no coercion during physical // planning. - binary(lhs, op.clone(), rhs, input_schema) + binary(lhs, *op, rhs, input_schema) } Expr::Like(Like { negated, diff --git a/datafusion/physical-expr/src/utils/mod.rs b/datafusion/physical-expr/src/utils/mod.rs index a33f65f92a61..6c4791b158c8 100644 --- a/datafusion/physical-expr/src/utils/mod.rs +++ b/datafusion/physical-expr/src/utils/mod.rs @@ -44,7 +44,7 @@ use petgraph::stable_graph::StableGraph; pub fn split_conjunction( predicate: &Arc, ) -> Vec<&Arc> { - split_impl(&Operator::And, predicate, vec![]) + split_impl(Operator::And, predicate, vec![]) } /// Assume the predicate is in the form of DNF, split the predicate to a Vec of PhysicalExprs. @@ -53,16 +53,16 @@ pub fn split_conjunction( pub fn split_disjunction( predicate: &Arc, ) -> Vec<&Arc> { - split_impl(&Operator::Or, predicate, vec![]) + split_impl(Operator::Or, predicate, vec![]) } fn split_impl<'a>( - operator: &Operator, + operator: Operator, predicate: &'a Arc, mut exprs: Vec<&'a Arc>, ) -> Vec<&'a Arc> { match predicate.as_any().downcast_ref::() { - Some(binary) if binary.op() == operator => { + Some(binary) if binary.op() == &operator => { let exprs = split_impl(operator, binary.left(), exprs); split_impl(operator, binary.right(), exprs) } diff --git a/datafusion/physical-plan/src/joins/hash_join.rs b/datafusion/physical-plan/src/joins/hash_join.rs index 2f4ee00da35f..16b3a4f2febd 100644 --- a/datafusion/physical-plan/src/joins/hash_join.rs +++ b/datafusion/physical-plan/src/joins/hash_join.rs @@ -1238,7 +1238,7 @@ fn eq_dyn_null( } else { Operator::Eq }; - return Ok(compare_op_for_nested(&op, &left, &right)?); + return Ok(compare_op_for_nested(op, &left, &right)?); } match (left.data_type(), right.data_type()) { _ if null_equals_null => not_distinct(&left, &right), diff --git a/datafusion/proto/src/logical_plan/from_proto.rs b/datafusion/proto/src/logical_plan/from_proto.rs index a58af8afdd04..095c6a50973a 100644 --- a/datafusion/proto/src/logical_plan/from_proto.rs +++ b/datafusion/proto/src/logical_plan/from_proto.rs @@ -269,11 +269,7 @@ pub fn parse_expr( Ok(operands .into_iter() .reduce(|left, right| { - Expr::BinaryExpr(BinaryExpr::new( - Box::new(left), - op.clone(), - Box::new(right), - )) + Expr::BinaryExpr(BinaryExpr::new(Box::new(left), op, Box::new(right))) }) .expect("Binary expression could not be reduced to a single expression.")) } diff --git a/datafusion/substrait/src/logical_plan/consumer.rs b/datafusion/substrait/src/logical_plan/consumer.rs index 77fd5fe44d44..89a6dde51e42 100644 --- a/datafusion/substrait/src/logical_plan/consumer.rs +++ b/datafusion/substrait/src/logical_plan/consumer.rs @@ -1154,7 +1154,7 @@ pub async fn from_substrait_rex( Arc::try_unwrap(expr) .unwrap_or_else(|arc: Arc| (*arc).clone()), ), // Avoid cloning if possible - op: op.clone(), + op, right: Box::new(arg), })), None => Arc::new(arg), diff --git a/datafusion/substrait/src/logical_plan/producer.rs b/datafusion/substrait/src/logical_plan/producer.rs index 959542080161..8d039a050249 100644 --- a/datafusion/substrait/src/logical_plan/producer.rs +++ b/datafusion/substrait/src/logical_plan/producer.rs @@ -664,12 +664,7 @@ fn to_substrait_join_expr( extension_info, )?; // AND with existing expression - exprs.push(make_binary_op_scalar_func( - &l, - &r, - eq_op.clone(), - extension_info, - )); + exprs.push(make_binary_op_scalar_func(&l, &r, eq_op, extension_info)); } let join_expr: Option = exprs.into_iter().reduce(|acc: Expression, e: Expression| { @@ -1167,12 +1162,7 @@ pub fn to_substrait_rex( let l = to_substrait_rex(ctx, left, schema, col_ref_offset, extension_info)?; let r = to_substrait_rex(ctx, right, schema, col_ref_offset, extension_info)?; - Ok(make_binary_op_scalar_func( - &l, - &r, - op.clone(), - extension_info, - )) + Ok(make_binary_op_scalar_func(&l, &r, *op, extension_info)) } Expr::Case(Case { expr,