peter-toth commented on PR #9913: URL: https://github.com/apache/arrow-datafusion/pull/9913#issuecomment-2041556063
> I took the liberty of restring `inspect_expressions` and marking it deprecated and merging up from main. Sure, thanks for doing that! > Removed the API change I was thinking about this a bit more and I unfortunately think this PR is still an API change. If some external code calls `.apply()` on a `LogicalPlan` then this PR might break it. This is because the implementation of `.apply()`will be de default implementation of `TreeNode::apply()` after this PR and no longer recurse into subqueries. The API user needs to change the code to `.apply_with_subqueries()` to get the old behaviour. But I have no other idea to fix the current inconsistency between `.apply()`/`.visit()`, that do recurse into subqueries and `.rewrite()`/`.transform()`, that don't. (Well we could change `.rewrite()`/`.transform()` to do recurse into subqueries, but that too would be a breaking change.) > I am not sure what you are planning on working on next @peter-toth but one thing I noticed is that keeping track of all the LogicalPlan tree node walking / rewriting is somewhat hard for me (and datafusion/expr/src/logical_plan/plan.rs is getting pretty large). I agree. I'm happy to take that task, but I will be travelling from Tuesday till the end of the week, so most likely I can do that once I'm back. -- 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]
