Skip to content

Commit 8c822cd

Browse files
Dandandanclaude
andcommitted
refactor: mutate current_exprs in place, drop alias clone
Addresses review feedback: build the substituted physical exprs into a slim Vec<Arc<dyn PhysicalExpr>>, then commit them in place so each ProjectionExpr's alias String is reused instead of cloned. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 00c0c23 commit 8c822cd

1 file changed

Lines changed: 9 additions & 7 deletions

File tree

datafusion/physical-plan/src/projection.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,21 +1048,23 @@ fn try_collapse_projection_chain(
10481048
break;
10491049
}
10501050

1051-
let mut new_exprs = Vec::with_capacity(current_exprs.len());
1051+
// Compute substituted exprs first, then commit them in place. A
1052+
// mid-loop bail would otherwise leave `current_exprs` half-updated
1053+
// and inconsistent with `current_input`.
1054+
let mut new_phys: Vec<Arc<dyn PhysicalExpr>> =
1055+
Vec::with_capacity(current_exprs.len());
10521056
for proj_expr in &current_exprs {
10531057
// If there is no match in the input projection, we cannot unify these
10541058
// projections. This case will arise if the projection expression contains
10551059
// a `PhysicalExpr` variant `update_expr` doesn't support.
10561060
let Some(expr) = update_expr(&proj_expr.expr, inner_exprs, true)? else {
10571061
break 'outer;
10581062
};
1059-
new_exprs.push(ProjectionExpr {
1060-
expr,
1061-
alias: proj_expr.alias.clone(),
1062-
});
1063+
new_phys.push(expr);
1064+
}
1065+
for (proj_expr, expr) in current_exprs.iter_mut().zip(new_phys) {
1066+
proj_expr.expr = expr;
10631067
}
1064-
1065-
current_exprs = new_exprs;
10661068
current_input = Arc::clone(inner_proj.input());
10671069
collapsed_any = true;
10681070
}

0 commit comments

Comments
 (0)