LiaCastaneda commented on issue #23814: URL: https://github.com/apache/datafusion/issues/23814#issuecomment-5092218448
@jayshrivastava if you decide to go with the generic function approach ( `apply_expressions`) will you need the write one as well: `map_expressions`? > ^ That is describing the producer side only, right? We need the consumer side as well. IIUC for the `ExecutionPlan::dynamic_filter` approach, in theory consumers _can_ specify their dynamic filters as well because they are also `ExecutionPlan` no? or is there some consideration I'm missing? 🤔 I think the only "ugly" thing is that the consumer node will need to either store it's dynamic filters in a separate variable or downcast it's predicates to know which expressions are dynamic filters. -- > it cannot not have a default implementation, so every ExecutionPlan is forced to implement it What if we implement `apply_expressions` on all DF nodes (like the reverted PR) but leave the default implementation to return `unimplemented!() `(and keep it optional). So if there is code in DF that actually uses it in the future, the user will know and it wont just fail/lie silently? Although I guess this option is not clean either. > What I would suggest to make progress is finding some spot in the DataFusion code that could be rewritten to use apply_expressions. I think there might be a few places in DF that could be rewritten to use `apply_expressions`, possibly some optimizer passes that downcast heavily on each node -- 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]
