alamb opened a new pull request, #7396: URL: https://github.com/apache/arrow-datafusion/pull/7396
## Which issue does this PR close? related to https://github.com/apache/arrow-datafusion/pull/7359 ## Rationale for this change While reiewing a great documentation improvement from @tshauck it took me a while to find `from_plan` used to rewrite LogicalPlans. See https://github.com/apache/arrow-datafusion/pull/7359#discussion_r1304408534 I think this makes it harder to work with the DataFusion codebase unecessairly The reason it took me a while to find I think was 1. Isn't method on `LogicalPlan` (it is in a utility module). 2. It didn't have `expr` or `exprs` in its name Thus, given it takes a `&LogicalPlan` as its first input I think it makes sense and would be more easily findable as a method on `LogicalPlan`. ## What changes are included in this PR? 1. Move code to create new `LogicalPlan`s with rewrtitten apis to `LogicalPlan::with_new_exprs()` 4. deprecate `from_plan` 3. I also noticed it often would force a copy of Expr's twice given its API so I made `LogicalPlan::new_with_exprs` take a `Vec<Expr>` instead of `&[Expr]` as it needs them owned anyways ## Are these changes tested? By existing tests ## Are there any user-facing changes? New API, deprecated old one -- 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]
