adriangb commented on PR #24601: URL: https://github.com/apache/datafusion/pull/24601#issuecomment-5396686803
> I like the solution, it's better than having this decision made at execution time. I guess the only caveat is that this rule will always have to run last. However, if we add a custom optimizer rule, isn't it appended after the built in ones? So the custom rule would run after `SanityCheckPlan`, isn't that an issue? Having to run last depends on what the rule being inserted it wants and how they interact. Btw this is already be the case for other operators that do things this way, for example AggregateExec. If a new rule wants to add a new consumer in a way that doesn't just transfer/copy the filters from the existing consumer then yes they'd need to run the filter pushdown optimizer again. I think that's a reasonable thing to do. But this is also the case today. If the rule wants to replace the scan with a new operator and is happy to copy the filters (as @jayshrivastava / datafusion-distributed want, or at least I understand they want) then there is no need to re-run filter pushdown. Can you think of any usage examples (even if somewhat artificial) where moving this check from execute() to planning time breaks the use case? -- 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]
