Skip to content

Commit

Permalink
Enable clone_on_ref_ptr clippy lint on optimizer (apache#11346)
Browse files Browse the repository at this point in the history
* Enable clone_on_ref_ptr clippy lint on optimizer

* Fix lints

* fmt
  • Loading branch information
lewiszlw authored and findepi committed Jul 16, 2024
1 parent 1844c80 commit dbe9750
Show file tree
Hide file tree
Showing 23 changed files with 73 additions and 64 deletions.
5 changes: 3 additions & 2 deletions datafusion/optimizer/src/analyzer/subquery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// under the License.

use std::ops::Deref;
use std::sync::Arc;

use crate::analyzer::check_plan;
use crate::utils::collect_subquery_cols;
Expand Down Expand Up @@ -245,7 +246,7 @@ fn check_aggregation_in_scalar_subquery(
if !agg.group_expr.is_empty() {
let correlated_exprs = get_correlated_expressions(inner_plan)?;
let inner_subquery_cols =
collect_subquery_cols(&correlated_exprs, agg.input.schema().clone())?;
collect_subquery_cols(&correlated_exprs, Arc::clone(agg.input.schema()))?;
let mut group_columns = agg
.group_expr
.iter()
Expand Down Expand Up @@ -375,7 +376,7 @@ mod test {
_inputs: Vec<LogicalPlan>,
) -> Result<Self> {
Ok(Self {
empty_schema: self.empty_schema.clone(),
empty_schema: Arc::clone(&self.empty_schema),
})
}
}
Expand Down
8 changes: 5 additions & 3 deletions datafusion/optimizer/src/analyzer/type_coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ fn coerce_arguments_for_fun(
.map(|expr| {
let data_type = expr.get_type(schema).unwrap();
if let DataType::FixedSizeList(field, _) = data_type {
let to_type = DataType::List(field.clone());
let to_type = DataType::List(Arc::clone(&field));
expr.cast_to(&to_type, schema)
} else {
Ok(expr)
Expand Down Expand Up @@ -1265,8 +1265,10 @@ mod test {
signature: Signature::variadic(vec![Utf8], Volatility::Immutable),
})
.call(args.to_vec());
let plan =
LogicalPlan::Projection(Projection::try_new(vec![expr], empty.clone())?);
let plan = LogicalPlan::Projection(Projection::try_new(
vec![expr],
Arc::clone(&empty),
)?);
let expected =
"Projection: TestScalarUDF(a, Utf8(\"b\"), CAST(Boolean(true) AS Utf8), CAST(Boolean(false) AS Utf8), CAST(Int32(13) AS Utf8))\n EmptyRelation";
assert_analyzed_plan_eq(Arc::new(TypeCoercion::new()), plan, expected)?;
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/common_subexpr_eliminate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1385,7 +1385,7 @@ mod test {
"my_agg",
Signature::exact(vec![DataType::UInt32], Volatility::Stable),
return_type.clone(),
accumulator.clone(),
Arc::clone(&accumulator),
vec![Field::new("value", DataType::UInt32, true)],
))),
vec![inner],
Expand Down
15 changes: 8 additions & 7 deletions datafusion/optimizer/src/decorrelate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use std::collections::{BTreeSet, HashMap};
use std::ops::Deref;
use std::sync::Arc;

use crate::simplify_expressions::ExprSimplifier;
use crate::utils::collect_subquery_cols;
Expand Down Expand Up @@ -147,7 +148,7 @@ impl TreeNodeRewriter for PullUpCorrelatedExpr {
}

fn f_up(&mut self, plan: LogicalPlan) -> Result<Transformed<LogicalPlan>> {
let subquery_schema = plan.schema().clone();
let subquery_schema = Arc::clone(plan.schema());
match &plan {
LogicalPlan::Filter(plan_filter) => {
let subquery_filter_exprs = split_conjunction(&plan_filter.predicate);
Expand All @@ -172,7 +173,7 @@ impl TreeNodeRewriter for PullUpCorrelatedExpr {
if let Some(expr) = conjunction(subquery_filters.clone()) {
filter_exprs_evaluation_result_on_empty_batch(
&expr,
plan_filter.input.schema().clone(),
Arc::clone(plan_filter.input.schema()),
expr_result_map,
&mut expr_result_map_for_count_bug,
)?
Expand Down Expand Up @@ -230,7 +231,7 @@ impl TreeNodeRewriter for PullUpCorrelatedExpr {
{
proj_exprs_evaluation_result_on_empty_batch(
&projection.expr,
projection.input.schema().clone(),
Arc::clone(projection.input.schema()),
expr_result_map,
&mut expr_result_map_for_count_bug,
)?;
Expand Down Expand Up @@ -276,7 +277,7 @@ impl TreeNodeRewriter for PullUpCorrelatedExpr {
{
agg_exprs_evaluation_result_on_empty_batch(
&aggregate.aggr_expr,
aggregate.input.schema().clone(),
Arc::clone(aggregate.input.schema()),
&mut expr_result_map_for_count_bug,
)?;
if !expr_result_map_for_count_bug.is_empty() {
Expand Down Expand Up @@ -332,7 +333,7 @@ impl TreeNodeRewriter for PullUpCorrelatedExpr {
if limit.fetch.filter(|limit_row| *limit_row == 0).is_some() {
LogicalPlan::EmptyRelation(EmptyRelation {
produce_one_row: false,
schema: limit.input.schema().clone(),
schema: Arc::clone(limit.input.schema()),
})
} else {
LogicalPlanBuilder::from((*limit.input).clone()).build()?
Expand Down Expand Up @@ -456,7 +457,7 @@ fn agg_exprs_evaluation_result_on_empty_batch(

let result_expr = result_expr.unalias();
let props = ExecutionProps::new();
let info = SimplifyContext::new(&props).with_schema(schema.clone());
let info = SimplifyContext::new(&props).with_schema(Arc::clone(&schema));
let simplifier = ExprSimplifier::new(info);
let result_expr = simplifier.simplify(result_expr)?;
if matches!(result_expr, Expr::Literal(ScalarValue::Int64(_))) {
Expand Down Expand Up @@ -492,7 +493,7 @@ fn proj_exprs_evaluation_result_on_empty_batch(

if result_expr.ne(expr) {
let props = ExecutionProps::new();
let info = SimplifyContext::new(&props).with_schema(schema.clone());
let info = SimplifyContext::new(&props).with_schema(Arc::clone(&schema));
let simplifier = ExprSimplifier::new(info);
let result_expr = simplifier.simplify(result_expr)?;
let expr_name = match expr {
Expand Down
4 changes: 2 additions & 2 deletions datafusion/optimizer/src/decorrelate_predicate_subquery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ mod tests {
);
let plan = LogicalPlanBuilder::from(scan_tpch_table("customer"))
.filter(
in_subquery(col("customer.c_custkey"), orders.clone())
in_subquery(col("customer.c_custkey"), Arc::clone(&orders))
.and(in_subquery(col("customer.c_custkey"), orders)),
)?
.project(vec![col("customer.c_custkey")])?
Expand Down Expand Up @@ -1358,7 +1358,7 @@ mod tests {
);

let plan = LogicalPlanBuilder::from(scan_tpch_table("customer"))
.filter(exists(orders.clone()).and(exists(orders)))?
.filter(exists(Arc::clone(&orders)).and(exists(orders)))?
.project(vec![col("customer.c_custkey")])?
.build()?;

Expand Down
6 changes: 3 additions & 3 deletions datafusion/optimizer/src/eliminate_cross_join.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl OptimizerRule for EliminateCrossJoin {
plan: LogicalPlan,
config: &dyn OptimizerConfig,
) -> Result<Transformed<LogicalPlan>> {
let plan_schema = plan.schema().clone();
let plan_schema = Arc::clone(plan.schema());
let mut possible_join_keys = JoinKeySet::new();
let mut all_inputs: Vec<LogicalPlan> = vec![];

Expand Down Expand Up @@ -155,7 +155,7 @@ impl OptimizerRule for EliminateCrossJoin {
if &plan_schema != left.schema() {
left = LogicalPlan::Projection(Projection::new_from_schema(
Arc::new(left),
plan_schema.clone(),
Arc::clone(&plan_schema),
));
}

Expand Down Expand Up @@ -420,7 +420,7 @@ mod tests {
};

fn assert_optimized_plan_eq(plan: LogicalPlan, expected: Vec<&str>) {
let starting_schema = plan.schema().clone();
let starting_schema = Arc::clone(plan.schema());
let rule = EliminateCrossJoin::new();
let transformed_plan = rule.rewrite(plan, &OptimizerContext::new()).unwrap();
assert!(transformed_plan.transformed, "failed to optimize plan");
Expand Down
3 changes: 2 additions & 1 deletion datafusion/optimizer/src/eliminate_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use datafusion_common::tree_node::Transformed;
use datafusion_common::{Result, ScalarValue};
use datafusion_expr::logical_plan::tree_node::unwrap_arc;
use datafusion_expr::{EmptyRelation, Expr, Filter, LogicalPlan};
use std::sync::Arc;

use crate::optimizer::ApplyOrder;
use crate::{OptimizerConfig, OptimizerRule};
Expand Down Expand Up @@ -68,7 +69,7 @@ impl OptimizerRule for EliminateFilter {
Some(false) | None => Ok(Transformed::yes(LogicalPlan::EmptyRelation(
EmptyRelation {
produce_one_row: false,
schema: input.schema().clone(),
schema: Arc::clone(input.schema()),
},
))),
},
Expand Down
3 changes: 2 additions & 1 deletion datafusion/optimizer/src/eliminate_limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::{OptimizerConfig, OptimizerRule};
use datafusion_common::tree_node::Transformed;
use datafusion_common::Result;
use datafusion_expr::logical_plan::{tree_node::unwrap_arc, EmptyRelation, LogicalPlan};
use std::sync::Arc;

/// Optimizer rule to replace `LIMIT 0` or `LIMIT` whose ancestor LIMIT's skip is
/// greater than or equal to current's fetch
Expand Down Expand Up @@ -67,7 +68,7 @@ impl OptimizerRule for EliminateLimit {
return Ok(Transformed::yes(LogicalPlan::EmptyRelation(
EmptyRelation {
produce_one_row: false,
schema: limit.input.schema().clone(),
schema: Arc::clone(limit.input.schema()),
},
)));
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_nested_union.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl OptimizerRule for EliminateNestedUnion {
Ok(Transformed::yes(LogicalPlan::Distinct(Distinct::All(
Arc::new(LogicalPlan::Union(Union {
inputs: inputs.into_iter().map(Arc::new).collect_vec(),
schema: schema.clone(),
schema: Arc::clone(&schema),
})),
))))
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_one_union.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ mod tests {
&table_scan(Some("table"), &schema(), None)?.build()?,
&schema().to_dfschema()?,
)?;
let schema = table_plan.schema().clone();
let schema = Arc::clone(table_plan.schema());
let single_union_plan = LogicalPlan::Union(Union {
inputs: vec![Arc::new(table_plan)],
schema,
Expand Down
2 changes: 1 addition & 1 deletion datafusion/optimizer/src/eliminate_outer_join.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl OptimizerRule for EliminateOuterJoin {
join_constraint: join.join_constraint,
on: join.on.clone(),
filter: join.filter.clone(),
schema: join.schema.clone(),
schema: Arc::clone(&join.schema),
null_equals_null: join.null_equals_null,
}));
Filter::try_new(filter.predicate, new_join)
Expand Down
4 changes: 2 additions & 2 deletions datafusion/optimizer/src/extract_equijoin_predicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,8 @@ mod tests {
let t1 = test_table_scan_with_name("t1")?;
let t2 = test_table_scan_with_name("t2")?;

let t1_schema = t1.schema().clone();
let t2_schema = t2.schema().clone();
let t1_schema = Arc::clone(t1.schema());
let t2_schema = Arc::clone(t2.schema());

// filter: t1.a + CAST(Int64(1), UInt32) = t2.a + CAST(Int64(2), UInt32) as t1.a + 1 = t2.a + 2
let filter = Expr::eq(
Expand Down
2 changes: 2 additions & 0 deletions datafusion/optimizer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
// Make cheap clones clear: https://github.com/apache/datafusion/issues/11143
#![deny(clippy::clone_on_ref_ptr)]

//! # DataFusion Optimizer
//!
Expand Down
12 changes: 6 additions & 6 deletions datafusion/optimizer/src/optimize_projections/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ fn optimize_projections(
});
}
LogicalPlan::Window(window) => {
let input_schema = window.input.schema().clone();
let input_schema = Arc::clone(window.input.schema());
// Split parent requirements to child and window expression sections:
let n_input_fields = input_schema.fields().len();
// Offset window expression indices so that they point to valid
Expand Down Expand Up @@ -881,7 +881,7 @@ mod tests {
Ok(Self {
exprs,
input: Arc::new(inputs.swap_remove(0)),
schema: self.schema.clone(),
schema: Arc::clone(&self.schema),
})
}

Expand Down Expand Up @@ -949,7 +949,7 @@ mod tests {
exprs,
left_child: Arc::new(inputs.remove(0)),
right_child: Arc::new(inputs.remove(0)),
schema: self.schema.clone(),
schema: Arc::clone(&self.schema),
})
}

Expand Down Expand Up @@ -1256,7 +1256,7 @@ mod tests {
let table_scan = test_table_scan()?;
let custom_plan = LogicalPlan::Extension(Extension {
node: Arc::new(NoOpUserDefined::new(
table_scan.schema().clone(),
Arc::clone(table_scan.schema()),
Arc::new(table_scan.clone()),
)),
});
Expand All @@ -1281,7 +1281,7 @@ mod tests {
let custom_plan = LogicalPlan::Extension(Extension {
node: Arc::new(
NoOpUserDefined::new(
table_scan.schema().clone(),
Arc::clone(table_scan.schema()),
Arc::new(table_scan.clone()),
)
.with_exprs(exprs),
Expand Down Expand Up @@ -1316,7 +1316,7 @@ mod tests {
let custom_plan = LogicalPlan::Extension(Extension {
node: Arc::new(
NoOpUserDefined::new(
table_scan.schema().clone(),
Arc::clone(table_scan.schema()),
Arc::new(table_scan.clone()),
)
.with_exprs(exprs),
Expand Down
8 changes: 4 additions & 4 deletions datafusion/optimizer/src/optimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ impl OptimizerConfig for OptimizerContext {
}

fn alias_generator(&self) -> Arc<AliasGenerator> {
self.alias_generator.clone()
Arc::clone(&self.alias_generator)
}

fn options(&self) -> &ConfigOptions {
Expand Down Expand Up @@ -381,7 +381,7 @@ impl Optimizer {
.skip_failed_rules
.then(|| new_plan.clone());

let starting_schema = new_plan.schema().clone();
let starting_schema = Arc::clone(new_plan.schema());

let result = match rule.apply_order() {
// optimizer handles recursion
Expand Down Expand Up @@ -579,7 +579,7 @@ mod tests {
let config = OptimizerContext::new().with_skip_failing_rules(false);

let input = Arc::new(test_table_scan()?);
let input_schema = input.schema().clone();
let input_schema = Arc::clone(input.schema());

let plan = LogicalPlan::Projection(Projection::try_new_with_schema(
vec![col("a"), col("b"), col("c")],
Expand Down Expand Up @@ -760,7 +760,7 @@ mod tests {
}

Ok(Transformed::yes(LogicalPlan::Projection(
Projection::try_new(exprs, projection.input.clone())?,
Projection::try_new(exprs, Arc::clone(&projection.input))?,
)))
}
}
Expand Down
4 changes: 2 additions & 2 deletions datafusion/optimizer/src/plan_signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ mod tests {
let one_node_plan =
Arc::new(LogicalPlan::EmptyRelation(datafusion_expr::EmptyRelation {
produce_one_row: false,
schema: schema.clone(),
schema: Arc::clone(&schema),
}));

assert_eq!(1, get_node_number(&one_node_plan).get());
Expand All @@ -112,7 +112,7 @@ mod tests {
assert_eq!(2, get_node_number(&two_node_plan).get());

let five_node_plan = Arc::new(LogicalPlan::Union(datafusion_expr::Union {
inputs: vec![two_node_plan.clone(), two_node_plan],
inputs: vec![Arc::clone(&two_node_plan), two_node_plan],
schema,
}));

Expand Down
Loading

0 comments on commit dbe9750

Please sign in to comment.