Skip to content

Commit

Permalink
Minor: remove clone in exprlist_to_fields (#9657)
Browse files Browse the repository at this point in the history
* remove clone

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

* remove lifetime

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

* fmt

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

---------

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
  • Loading branch information
jayzhan211 committed Mar 18, 2024
1 parent e37ac35 commit cab5d0f
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 10 deletions.
9 changes: 5 additions & 4 deletions datafusion/expr/src/logical_plan/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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(),
)?;

Expand Down Expand Up @@ -2436,7 +2437,7 @@ impl Aggregate {

let grouping_expr: Vec<Expr> = 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 {
Expand All @@ -2446,7 +2447,7 @@ impl Aggregate {
.collect::<Vec<_>>();
}

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())?;
Expand Down
8 changes: 2 additions & 6 deletions datafusion/expr/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,17 +753,13 @@ fn exprlist_to_fields_aggregate(exprs: &[Expr], agg: &Aggregate) -> Result<Vec<D
}

/// Create field meta-data from an expression, for use in a result set schema
pub fn exprlist_to_fields<'a>(
expr: impl IntoIterator<Item = &'a Expr>,
plan: &LogicalPlan,
) -> Result<Vec<DFField>> {
let exprs: Vec<Expr> = expr.into_iter().cloned().collect();
pub fn exprlist_to_fields(exprs: &[Expr], plan: &LogicalPlan) -> Result<Vec<DFField>> {
// 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 {
Expand Down

0 comments on commit cab5d0f

Please sign in to comment.