From 5aa7c4ae7977cd043f28c4f55e07fa72d278a7b7 Mon Sep 17 00:00:00 2001 From: Jay Zhan Date: Mon, 8 Jul 2024 03:52:31 +0800 Subject: [PATCH] Minor: Remove clone in optimizer (#11315) * rm clone Signed-off-by: jayzhan211 * outer join + fix test Signed-off-by: jayzhan211 --------- Signed-off-by: jayzhan211 --- .../optimizer/src/eliminate_nested_union.rs | 34 +++++++++++-------- .../optimizer/src/eliminate_outer_join.rs | 13 ++++--- .../optimizer/src/propagate_empty_relation.rs | 4 ++- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/datafusion/optimizer/src/eliminate_nested_union.rs b/datafusion/optimizer/src/eliminate_nested_union.rs index 09407aed53cd..3732f7ed90c8 100644 --- a/datafusion/optimizer/src/eliminate_nested_union.rs +++ b/datafusion/optimizer/src/eliminate_nested_union.rs @@ -21,7 +21,9 @@ use crate::{OptimizerConfig, OptimizerRule}; use datafusion_common::tree_node::Transformed; use datafusion_common::Result; use datafusion_expr::expr_rewriter::coerce_plan_expr_for_schema; +use datafusion_expr::logical_plan::tree_node::unwrap_arc; use datafusion_expr::{Distinct, LogicalPlan, Union}; +use itertools::Itertools; use std::sync::Arc; #[derive(Default)] @@ -56,32 +58,34 @@ impl OptimizerRule for EliminateNestedUnion { match plan { LogicalPlan::Union(Union { inputs, schema }) => { let inputs = inputs - .iter() + .into_iter() .flat_map(extract_plans_from_union) .collect::>(); Ok(Transformed::yes(LogicalPlan::Union(Union { - inputs, + inputs: inputs.into_iter().map(Arc::new).collect_vec(), schema, }))) } - LogicalPlan::Distinct(Distinct::All(ref nested_plan)) => { - match nested_plan.as_ref() { + LogicalPlan::Distinct(Distinct::All(nested_plan)) => { + match unwrap_arc(nested_plan) { LogicalPlan::Union(Union { inputs, schema }) => { let inputs = inputs - .iter() + .into_iter() .map(extract_plan_from_distinct) .flat_map(extract_plans_from_union) .collect::>(); Ok(Transformed::yes(LogicalPlan::Distinct(Distinct::All( Arc::new(LogicalPlan::Union(Union { - inputs, + inputs: inputs.into_iter().map(Arc::new).collect_vec(), schema: schema.clone(), })), )))) } - _ => Ok(Transformed::no(plan)), + nested_plan => Ok(Transformed::no(LogicalPlan::Distinct( + Distinct::All(Arc::new(nested_plan)), + ))), } } _ => Ok(Transformed::no(plan)), @@ -89,20 +93,20 @@ impl OptimizerRule for EliminateNestedUnion { } } -fn extract_plans_from_union(plan: &Arc) -> Vec> { - match plan.as_ref() { +fn extract_plans_from_union(plan: Arc) -> Vec { + match unwrap_arc(plan) { LogicalPlan::Union(Union { inputs, schema }) => inputs - .iter() - .map(|plan| Arc::new(coerce_plan_expr_for_schema(plan, schema).unwrap())) + .into_iter() + .map(|plan| coerce_plan_expr_for_schema(&plan, &schema).unwrap()) .collect::>(), - _ => vec![plan.clone()], + plan => vec![plan], } } -fn extract_plan_from_distinct(plan: &Arc) -> &Arc { - match plan.as_ref() { +fn extract_plan_from_distinct(plan: Arc) -> Arc { + match unwrap_arc(plan) { LogicalPlan::Distinct(Distinct::All(plan)) => plan, - _ => plan, + plan => Arc::new(plan), } } diff --git a/datafusion/optimizer/src/eliminate_outer_join.rs b/datafusion/optimizer/src/eliminate_outer_join.rs index ccc637a0eb01..13c483c6dfcc 100644 --- a/datafusion/optimizer/src/eliminate_outer_join.rs +++ b/datafusion/optimizer/src/eliminate_outer_join.rs @@ -18,6 +18,7 @@ //! [`EliminateOuterJoin`] converts `LEFT/RIGHT/FULL` joins to `INNER` joins use crate::{OptimizerConfig, OptimizerRule}; use datafusion_common::{Column, DFSchema, Result}; +use datafusion_expr::logical_plan::tree_node::unwrap_arc; use datafusion_expr::logical_plan::{Join, JoinType, LogicalPlan}; use datafusion_expr::{Expr, Filter, Operator}; @@ -78,7 +79,7 @@ impl OptimizerRule for EliminateOuterJoin { _config: &dyn OptimizerConfig, ) -> Result> { match plan { - LogicalPlan::Filter(filter) => match filter.input.as_ref() { + LogicalPlan::Filter(mut filter) => match unwrap_arc(filter.input) { LogicalPlan::Join(join) => { let mut non_nullable_cols: Vec = vec![]; @@ -109,9 +110,10 @@ impl OptimizerRule for EliminateOuterJoin { } else { join.join_type }; + let new_join = Arc::new(LogicalPlan::Join(Join { - left: Arc::new((*join.left).clone()), - right: Arc::new((*join.right).clone()), + left: join.left, + right: join.right, join_type: new_join_type, join_constraint: join.join_constraint, on: join.on.clone(), @@ -122,7 +124,10 @@ impl OptimizerRule for EliminateOuterJoin { Filter::try_new(filter.predicate, new_join) .map(|f| Transformed::yes(LogicalPlan::Filter(f))) } - _ => Ok(Transformed::no(LogicalPlan::Filter(filter))), + filter_input => { + filter.input = Arc::new(filter_input); + Ok(Transformed::no(LogicalPlan::Filter(filter))) + } }, _ => Ok(Transformed::no(plan)), } diff --git a/datafusion/optimizer/src/propagate_empty_relation.rs b/datafusion/optimizer/src/propagate_empty_relation.rs index 576dabe305e6..88bd1b17883b 100644 --- a/datafusion/optimizer/src/propagate_empty_relation.rs +++ b/datafusion/optimizer/src/propagate_empty_relation.rs @@ -182,7 +182,9 @@ impl OptimizerRule for PropagateEmptyRelation { }, ))) } else if new_inputs.len() == 1 { - let child = unwrap_arc(new_inputs[0].clone()); + let mut new_inputs = new_inputs; + let input_plan = new_inputs.pop().unwrap(); // length checked + let child = unwrap_arc(input_plan); if child.schema().eq(plan.schema()) { Ok(Transformed::yes(child)) } else {