berkaysynnada commented on PR #15566: URL: https://github.com/apache/datafusion/pull/15566#issuecomment-2792250700
@adriangb I don’t want you to go back and forth excessively. Since you’ve spent a lot of time on this and are supporting the current version as the easiest and most understandable way to extend, I’m fine with following your decision. It seems there are pros/cons in both approaches (though I’m still in favor of rule recursion and API separation), but we need to move forward by picking one. I don’t think switching to the other mechanism would require much effort, so I might try it out and see how it ends up (but I can’t spend time on that right now unfortunately). I don’t know if this makes sense to you, but here’s one last design suggestion: What if we define a new trait like `DynamicFilterPlan: ExecutionPlan` and add all the required APIs to it? Then the main driver logic in the optimizer could check whether a plan is just an ExecutionPlan (handled by the default logic, possibly configurable for transparent operators too), or if it’s a DynamicFilterPlan, which has all the necessary APIs implemented to allow continued pushdown. TLDR, I’ve made one last design suggestion above. If it doesn’t make sense or isn’t applicable, feel free to ignore it. We can merge this as is. This is a great PR, and what we’ve been discussing isn’t a major issue either way. Thank you again -- 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
