Skip to content

Commit 2def10f

Browse files
authored
Stop copying plans in LogicalPlan::with_param_values (#10016)
1 parent 118eecd commit 2def10f

File tree

1 file changed

+28
-51
lines changed
  • datafusion/expr/src/logical_plan

1 file changed

+28
-51
lines changed

datafusion/expr/src/logical_plan/plan.rs

Lines changed: 28 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -913,14 +913,19 @@ impl LogicalPlan {
913913
param_values: impl Into<ParamValues>,
914914
) -> Result<LogicalPlan> {
915915
let param_values = param_values.into();
916-
match self {
917-
LogicalPlan::Prepare(prepare_lp) => {
918-
param_values.verify(&prepare_lp.data_types)?;
919-
let input_plan = prepare_lp.input;
920-
input_plan.replace_params_with_values(&param_values)
916+
let plan_with_values = self.replace_params_with_values(&param_values)?;
917+
918+
// unwrap Prepare
919+
Ok(if let LogicalPlan::Prepare(prepare_lp) = plan_with_values {
920+
param_values.verify(&prepare_lp.data_types)?;
921+
// try and take ownership of the input if is not shared, clone otherwise
922+
match Arc::try_unwrap(prepare_lp.input) {
923+
Ok(input) => input,
924+
Err(arc_input) => arc_input.as_ref().clone(),
921925
}
922-
_ => self.replace_params_with_values(&param_values),
923-
}
926+
} else {
927+
plan_with_values
928+
})
924929
}
925930

926931
/// Returns the maximum number of rows that this plan can output, if known.
@@ -1046,27 +1051,26 @@ impl LogicalPlan {
10461051
/// ...) replaced with corresponding values provided in
10471052
/// `params_values`
10481053
///
1049-
/// See [`Self::with_param_values`] for examples and usage
1054+
/// See [`Self::with_param_values`] for examples and usage with an owned
1055+
/// `ParamValues`
10501056
pub fn replace_params_with_values(
1051-
&self,
1057+
self,
10521058
param_values: &ParamValues,
10531059
) -> Result<LogicalPlan> {
1054-
let new_exprs = self
1055-
.expressions()
1056-
.into_iter()
1057-
.map(|e| {
1058-
let e = e.infer_placeholder_types(self.schema())?;
1059-
Self::replace_placeholders_with_values(e, param_values)
1060+
self.transform_up_with_subqueries(&|plan| {
1061+
let schema = plan.schema().clone();
1062+
plan.map_expressions(|e| {
1063+
e.infer_placeholder_types(&schema)?.transform_up(&|e| {
1064+
if let Expr::Placeholder(Placeholder { id, .. }) = e {
1065+
let value = param_values.get_placeholders_with_values(&id)?;
1066+
Ok(Transformed::yes(Expr::Literal(value)))
1067+
} else {
1068+
Ok(Transformed::no(e))
1069+
}
1070+
})
10601071
})
1061-
.collect::<Result<Vec<_>>>()?;
1062-
1063-
let new_inputs_with_values = self
1064-
.inputs()
1065-
.into_iter()
1066-
.map(|inp| inp.replace_params_with_values(param_values))
1067-
.collect::<Result<Vec<_>>>()?;
1068-
1069-
self.with_new_exprs(new_exprs, new_inputs_with_values)
1072+
})
1073+
.map(|res| res.data)
10701074
}
10711075

10721076
/// Walk the logical plan, find any `Placeholder` tokens, and return a map of their IDs and DataTypes
@@ -1099,33 +1103,6 @@ impl LogicalPlan {
10991103
.map(|_| param_types)
11001104
}
11011105

1102-
/// Return an Expr with all placeholders replaced with their
1103-
/// corresponding values provided in the params_values
1104-
fn replace_placeholders_with_values(
1105-
expr: Expr,
1106-
param_values: &ParamValues,
1107-
) -> Result<Expr> {
1108-
expr.transform(&|expr| {
1109-
match &expr {
1110-
Expr::Placeholder(Placeholder { id, .. }) => {
1111-
let value = param_values.get_placeholders_with_values(id)?;
1112-
// Replace the placeholder with the value
1113-
Ok(Transformed::yes(Expr::Literal(value)))
1114-
}
1115-
Expr::ScalarSubquery(qry) => {
1116-
let subquery =
1117-
Arc::new(qry.subquery.replace_params_with_values(param_values)?);
1118-
Ok(Transformed::yes(Expr::ScalarSubquery(Subquery {
1119-
subquery,
1120-
outer_ref_columns: qry.outer_ref_columns.clone(),
1121-
})))
1122-
}
1123-
_ => Ok(Transformed::no(expr)),
1124-
}
1125-
})
1126-
.data()
1127-
}
1128-
11291106
// ------------
11301107
// Various implementations for printing out LogicalPlans
11311108
// ------------

0 commit comments

Comments
 (0)