kosiew commented on code in PR #23332:
URL: https://github.com/apache/datafusion/pull/23332#discussion_r3549378470
##########
datafusion/physical-plan/src/execution_plan.rs:
##########
@@ -235,6 +235,27 @@ pub trait ExecutionPlan: Any + Debug + DisplayAs + Send +
Sync {
children: Vec<Arc<dyn ExecutionPlan>>,
) -> Result<Arc<dyn ExecutionPlan>>;
+ /// Fast-path used by [`with_new_children_if_necessary`] when the new
+ /// `children` are known to have the same [`PlanProperties`] as the current
+ /// children. Implementations should swap the children in without
+ /// recomputing this plan's `PlanProperties` (typically by cloning `self`
+ /// and replacing the child pointers).
+ ///
+ /// The default implementation falls back to
+ /// [`ExecutionPlan::with_new_children`] which is always correct but
+ /// forfeits the fast-path: implementations that own an expensive
+ /// `PlanProperties` (e.g. projection mapping, complex equivalence
+ /// classes) should override this method.
+ ///
+ /// Callers should route through [`with_new_children_if_necessary`] and
+ /// not invoke this method directly.
+ fn with_new_children_and_same_properties(
Review Comment:
Nice change! One small suggestion: it might be worth adding a test for a
plan that does not override `with_new_children_and_same_properties`, where the
replacement child has the same `PlanProperties` `Arc`. That would help show
that the default fallback still goes through `with_new_children` and keeps
external or custom `ExecutionPlan` implementations on the semantics-preserving
path. Totally non-blocking though. The existing override-path coverage and
`cargo check` already look good.
--
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]
