JSOD11 commented on code in PR #23903:
URL: https://github.com/apache/datafusion/pull/23903#discussion_r3744749001
##########
datafusion/physical-plan/src/execution_plan.rs:
##########
@@ -308,13 +333,13 @@ pub trait ExecutionPlan: Any + Debug + DisplayAs + Send +
Sync {
/// it will be called from within a walk of the execution plan tree so
that it will be called on each child later
/// or was already called on each child.
///
- /// Note to implementers: unlike [`ExecutionPlan::with_new_children`] this
method does not accept new children as an argument,
+ /// Note to implementers: unlike [`ExecutionPlan::replace_children`] this
method does not accept new children as an argument,
/// thus it is expected that any cached plan properties will remain valid
after the reset.
///
/// [`DynamicFilterPhysicalExpr`]:
datafusion_physical_expr::expressions::DynamicFilterPhysicalExpr
fn reset_state(self: Arc<Self>) -> Result<Arc<dyn ExecutionPlan>> {
let children = self.children().into_iter().cloned().collect();
- self.with_new_children(children)
+ self.replace_children(children, ChildrenPropertiesHint::Recompute)
Review Comment:
Good call, agreed. Just pushed a commit swapping this to `SameProperties`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
