This is an automated email from the ASF dual-hosted git repository.
agrove pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new a6898d36c4 Revert "remove `derive(Copy)` from `Operator` (#11132)"
(#11341)
a6898d36c4 is described below
commit a6898d36c4bde7ab77d5cc912a6aff5eb620df77
Author: Andrew Lamb <[email protected]>
AuthorDate: Mon Jul 8 20:27:58 2024 -0400
Revert "remove `derive(Copy)` from `Operator` (#11132)" (#11341)
This reverts commit b468ba788310d5dcd6ea4549f0d5b0f215ea49c9.
---
datafusion/core/src/physical_optimizer/pruning.rs | 24 ++++++++--------------
datafusion/expr/src/operator.rs | 2 +-
datafusion/expr/src/type_coercion/binary.rs | 4 ++--
datafusion/expr/src/utils.rs | 22 ++++++++++----------
datafusion/optimizer/src/analyzer/type_coercion.rs | 8 ++++----
.../optimizer/src/simplify_expressions/utils.rs | 8 ++++----
datafusion/optimizer/src/utils.rs | 6 +++---
datafusion/physical-expr-common/src/datum.rs | 4 ++--
datafusion/physical-expr/src/expressions/binary.rs | 4 ++--
.../physical-expr/src/intervals/cp_solver.rs | 2 +-
datafusion/physical-expr/src/intervals/utils.rs | 2 +-
datafusion/physical-expr/src/planner.rs | 2 +-
datafusion/physical-expr/src/utils/mod.rs | 8 ++++----
datafusion/physical-plan/src/joins/hash_join.rs | 2 +-
datafusion/proto/src/logical_plan/from_proto.rs | 6 +-----
datafusion/substrait/src/logical_plan/consumer.rs | 2 +-
datafusion/substrait/src/logical_plan/producer.rs | 14 ++-----------
17 files changed, 50 insertions(+), 70 deletions(-)
diff --git a/datafusion/core/src/physical_optimizer/pruning.rs
b/datafusion/core/src/physical_optimizer/pruning.rs
index e8f2f34abd..a7ce29bdc7 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<dyn PhysicalExpr> {
@@ -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::<phys_expr::BinaryExpr>() {
(
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 742511822a..a10312e234 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 83a7da0468..4f79f3fa2b 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 34e0072074..45155cbd2c 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<Expr> {
- split_binary_owned(expr, &Operator::And)
+ split_binary_owned(expr, Operator::And)
}
/// Splits an owned binary operator tree [`Expr`] such as `A <OP> B <OP> C` =>
`[A, B, C]`
@@ -1020,19 +1020,19 @@ pub fn split_conjunction_owned(expr: Expr) -> Vec<Expr>
{
/// ];
///
/// // 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<Expr> {
+pub fn split_binary_owned(expr: Expr, op: Operator) -> Vec<Expr> {
split_binary_owned_impl(expr, op, vec![])
}
fn split_binary_owned_impl(
expr: Expr,
- operator: &Operator,
+ operator: Operator,
mut exprs: Vec<Expr>,
) -> Vec<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_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 <OP> B <OP> 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 64d9c508f3..6c08b3e998 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::<Result<Vec<_>>>()?;
@@ -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 ed3fd75f3e..5da727cb59 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 0549845430..05b1744d90 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<Expr> {
/// ];
///
/// // 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<Expr> {
+pub fn split_binary_owned(expr: Expr, op: Operator) -> Vec<Expr> {
expr_utils::split_binary_owned(expr, op)
}
@@ -194,7 +194,7 @@ pub fn split_binary_owned(expr: Expr, op: &Operator) ->
Vec<Expr> {
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 790e742c42..d0ba5f113b 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<ColumnarValue> {
@@ -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<BooleanArray> {
diff --git a/datafusion/physical-expr/src/expressions/binary.rs
b/datafusion/physical-expr/src/expressions/binary.rs
index f1e40575bc..c153ead963 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<Arc<dyn PhysicalExpr>> {
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 ef9dd36cfb..fc4950ae4e 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<Option<(Interval, Interval)>> {
- 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 37527802f8..b426a656fb 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<dyn PhysicalExpr>, schema:
&SchemaRef) -> bool {
}
// This function returns the inverse operator of the given operator.
-pub fn get_inverse_op(op: &Operator) -> Result<Operator> {
+pub fn get_inverse_op(op: Operator) -> Result<Operator> {
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 dbebf4c18b..a975f0c6ef 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 a33f65f92a..6c4791b158 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<dyn PhysicalExpr>,
) -> Vec<&Arc<dyn PhysicalExpr>> {
- 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<dyn PhysicalExpr>,
) -> Vec<&Arc<dyn PhysicalExpr>> {
- split_impl(&Operator::Or, predicate, vec![])
+ split_impl(Operator::Or, predicate, vec![])
}
fn split_impl<'a>(
- operator: &Operator,
+ operator: Operator,
predicate: &'a Arc<dyn PhysicalExpr>,
mut exprs: Vec<&'a Arc<dyn PhysicalExpr>>,
) -> Vec<&'a Arc<dyn PhysicalExpr>> {
match predicate.as_any().downcast_ref::<BinaryExpr>() {
- 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 2f4ee00da3..16b3a4f2fe 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 a58af8afdd..095c6a5097 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 77fd5fe44d..89a6dde51e 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<Expr>|
(*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 9595420801..8d039a0502 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<Expression> =
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,
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
