zhuqi-lucas opened a new issue, #23441: URL: https://github.com/apache/datafusion/issues/23441
## Follow-up to [#23332](https://github.com/apache/datafusion/pull/23332) Per [@alamb's review](https://github.com/apache/datafusion/pull/23332#discussion_r3554897693), landing PR #23332 leaves the `ExecutionPlan` trait with **two** methods that each build a new plan from new children: - `fn with_new_children(self: Arc<Self>, children) -> Result<...>` — always recomputes `PlanProperties`. - `fn with_new_children_and_same_properties(self: Arc<Self>, children) -> Result<...>` — reuses `PlanProperties`; default falls back to `with_new_children`. For someone implementing `ExecutionPlan`, seeing both is confusing ("which one do I override, and when?"). Long-term the trait should expose a single canonical method. ## Proposal 1. **Introduce a canonical entry point**, e.g. `replace_children(self: Arc<Self>, children, hint: ChildrenPropertiesHint) -> Result<Arc<dyn ExecutionPlan>>`, where the hint encodes "same properties, reuse cache" vs "recompute". 2. **Deprecate** both `with_new_children` and `with_new_children_and_same_properties` with clear migration notes pointing at `replace_children`. 3. **Migrate the ~30 in-tree `ExecutionPlan` impls** and all direct callers to the new method. 4. **After a couple of releases**, drop the deprecated methods, leaving the trait with a single canonical children-replacement primitive. The `with_new_children_if_necessary` helper stays the caller-facing API and internally routes to `replace_children` with the appropriate hint. ## Why not fold this into #23332 - The mechanical migration touches ~30 files; keeping it as a separate PR lets the semantic change (fast-path skip) land first and be reviewed on its own. - The API rename is a naming discussion worth its own review thread. ## Related - PR [#23332](https://github.com/apache/datafusion/pull/23332) — semantic change this issue builds on. - Umbrella [#22555](https://github.com/apache/datafusion/issues/22555) — `with_new_children_if_necessary` consolidation. cc @alamb -- 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]
