From cab5d0f35575b3723cc537de0a8f355c2193b903 Mon Sep 17 00:00:00 2001 From: Jay Zhan Date: Mon, 18 Mar 2024 19:30:36 +0800 Subject: [PATCH] Minor: remove clone in `exprlist_to_fields` (#9657) * remove clone Signed-off-by: jayzhan211 * remove lifetime Signed-off-by: jayzhan211 * fmt Signed-off-by: jayzhan211 --------- Signed-off-by: jayzhan211 --- datafusion/expr/src/logical_plan/plan.rs | 9 +++++---- datafusion/expr/src/utils.rs | 8 ++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs index a3f027d9fdb2..c6f280acb255 100644 --- a/datafusion/expr/src/logical_plan/plan.rs +++ b/datafusion/expr/src/logical_plan/plan.rs @@ -2031,7 +2031,8 @@ impl Window { let fields = input.schema().fields(); let input_len = fields.len(); let mut window_fields = fields.clone(); - window_fields.extend_from_slice(&exprlist_to_fields(window_expr.iter(), &input)?); + let expr_fields = exprlist_to_fields(window_expr.as_slice(), &input)?; + window_fields.extend_from_slice(expr_fields.as_slice()); let metadata = input.schema().metadata().clone(); // Update functional dependencies for window: @@ -2357,7 +2358,7 @@ impl DistinctOn { let on_expr = normalize_cols(on_expr, input.as_ref())?; let schema = DFSchema::new_with_metadata( - exprlist_to_fields(&select_expr, &input)?, + exprlist_to_fields(select_expr.as_slice(), &input)?, input.schema().metadata().clone(), )?; @@ -2436,7 +2437,7 @@ impl Aggregate { let grouping_expr: Vec = grouping_set_to_exprlist(group_expr.as_slice())?; - let mut fields = exprlist_to_fields(grouping_expr.iter(), &input)?; + let mut fields = exprlist_to_fields(grouping_expr.as_slice(), &input)?; // Even columns that cannot be null will become nullable when used in a grouping set. if is_grouping_set { @@ -2446,7 +2447,7 @@ impl Aggregate { .collect::>(); } - fields.extend(exprlist_to_fields(aggr_expr.iter(), &input)?); + fields.extend(exprlist_to_fields(aggr_expr.as_slice(), &input)?); let schema = DFSchema::new_with_metadata(fields, input.schema().metadata().clone())?; diff --git a/datafusion/expr/src/utils.rs b/datafusion/expr/src/utils.rs index dfd90e470965..c7907d0db16a 100644 --- a/datafusion/expr/src/utils.rs +++ b/datafusion/expr/src/utils.rs @@ -753,17 +753,13 @@ fn exprlist_to_fields_aggregate(exprs: &[Expr], agg: &Aggregate) -> Result( - expr: impl IntoIterator, - plan: &LogicalPlan, -) -> Result> { - let exprs: Vec = expr.into_iter().cloned().collect(); +pub fn exprlist_to_fields(exprs: &[Expr], plan: &LogicalPlan) -> Result> { // when dealing with aggregate plans we cannot simply look in the aggregate output schema // because it will contain columns representing complex expressions (such a column named // `GROUPING(person.state)` so in order to resolve `person.state` in this case we need to // look at the input to the aggregate instead. let fields = match plan { - LogicalPlan::Aggregate(agg) => Some(exprlist_to_fields_aggregate(&exprs, agg)), + LogicalPlan::Aggregate(agg) => Some(exprlist_to_fields_aggregate(exprs, agg)), _ => None, }; if let Some(fields) = fields {