From ac163116884acabca2d93e1c0797118553eb4ac2 Mon Sep 17 00:00:00 2001 From: remzi <13716567376yh@gmail.com> Date: Sun, 19 Mar 2023 13:56:32 +0800 Subject: [PATCH 1/2] update the function type Signed-off-by: remzi <13716567376yh@gmail.com> --- datafusion/expr/src/logical_plan/plan.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs index a6ba1c96154f..a129a8073e77 100644 --- a/datafusion/expr/src/logical_plan/plan.rs +++ b/datafusion/expr/src/logical_plan/plan.rs @@ -632,16 +632,16 @@ impl LogicalPlan { /// params_values pub fn replace_params_with_values( &self, - param_values: &Vec, + param_values: &[ScalarValue], ) -> Result { let exprs = self.expressions(); - let mut new_exprs = vec![]; + let mut new_exprs = Vec::with_capacity(exprs.len()); for expr in exprs { new_exprs.push(Self::replace_placeholders_with_values(expr, param_values)?); } let new_inputs = self.inputs(); - let mut new_inputs_with_values = vec![]; + let mut new_inputs_with_values = Vec::with_capacity(new_inputs.len()); for input in new_inputs { new_inputs_with_values.push(input.replace_params_with_values(param_values)?); } @@ -748,10 +748,8 @@ impl LogicalPlan { Ok(Expr::Literal(value.clone())) } Expr::ScalarSubquery(qry) => { - let subquery = Arc::new( - qry.subquery - .replace_params_with_values(¶m_values.to_vec())?, - ); + let subquery = + Arc::new(qry.subquery.replace_params_with_values(param_values)?); Ok(Expr::ScalarSubquery(plan::Subquery { subquery })) } _ => Ok(expr), From 4a861e87c094b8b44bf5c61d1b4ce2b8a9af9319 Mon Sep 17 00:00:00 2001 From: remzi <13716567376yh@gmail.com> Date: Sun, 19 Mar 2023 20:11:52 +0800 Subject: [PATCH 2/2] address comment Signed-off-by: remzi <13716567376yh@gmail.com> --- datafusion/expr/src/logical_plan/plan.rs | 26 +++++++++++------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs index a129a8073e77..5c93bb6f2d8b 100644 --- a/datafusion/expr/src/logical_plan/plan.rs +++ b/datafusion/expr/src/logical_plan/plan.rs @@ -24,8 +24,7 @@ use crate::logical_plan::display::{GraphvizVisitor, IndentVisitor}; use crate::logical_plan::extension::UserDefinedLogicalNode; use crate::logical_plan::plan; use crate::utils::{ - self, exprlist_to_fields, from_plan, grouping_set_expr_count, - grouping_set_to_exprlist, + exprlist_to_fields, from_plan, grouping_set_expr_count, grouping_set_to_exprlist, }; use crate::{ build_join_schema, Expr, ExprSchemable, TableProviderFilterPushDown, TableSource, @@ -634,20 +633,19 @@ impl LogicalPlan { &self, param_values: &[ScalarValue], ) -> Result { - let exprs = self.expressions(); - let mut new_exprs = Vec::with_capacity(exprs.len()); - for expr in exprs { - new_exprs.push(Self::replace_placeholders_with_values(expr, param_values)?); - } + let new_exprs = self + .expressions() + .into_iter() + .map(|e| Self::replace_placeholders_with_values(e, param_values)) + .collect::, DataFusionError>>()?; - let new_inputs = self.inputs(); - let mut new_inputs_with_values = Vec::with_capacity(new_inputs.len()); - for input in new_inputs { - new_inputs_with_values.push(input.replace_params_with_values(param_values)?); - } + let new_inputs_with_values = self + .inputs() + .into_iter() + .map(|inp| inp.replace_params_with_values(param_values)) + .collect::, DataFusionError>>()?; - let new_plan = utils::from_plan(self, &new_exprs, &new_inputs_with_values)?; - Ok(new_plan) + from_plan(self, &new_exprs, &new_inputs_with_values) } /// Walk the logical plan, find any `PlaceHolder` tokens, and return a map of their IDs and DataTypes