alamb commented on PR #10543: URL: https://github.com/apache/datafusion/pull/10543#issuecomment-2115952198
I agree with @berkaysynnada in https://github.com/apache/datafusion/pull/10543#issuecomment-2115389289 that in an ideal world we woul change `TreeNode::visit` and `TreeNode::apply`, however as @peter-toth notes this would: 1. Be a much larger change (and we would have to sort out how it works for `DynTreeNodes`) 2. be a breaking API change Thus, I suggest we merge this PR as is, and file a follow on PR to potentially unify the API. While it in an ideal world we wouldn't have both, I think this is a step in the right direction Is that ok with you @berkaysynnada ? -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
