jayshrivastava commented on issue #23814: URL: https://github.com/apache/datafusion/issues/23814#issuecomment-5096920422
> if you decide to go with the generic function approach (apply_expressions) will you need the write one as well: map_expressions? I don't think we need `map_expressions` because we should be able to call `apply_expressions`, downcast to `&DynamicFilterPhysicalExpr`, and call `update()`, which is `pub`. https://github.com/apache/datafusion/blob/3a29d6bd8cc9ac2bf5efee9f070dcdeea9f97b32/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs?plain=1#L246 We can read updates from `&DynamicFilterPhysicalExpr` via `wait_complete` https://github.com/apache/datafusion/blob/3a29d6bd8cc9ac2bf5efee9f070dcdeea9f97b32/datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs?plain=1#L320 > 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. Maybe `ExecutionPlan::dynamic_filters_produced` is more clear? The proposed contract is that consumers don't return them here. You make a good point about producers returning their expressions via `apply_expressions`. They probably should do that because `apply_expressions` is for all expressions. Maybe the caller (datafusion distributed or otherwise) can take the difference: `consumer_dynamic_filter_expressions = ExecutionPlan::apply_expressions() - ExecutionPlan::dynamic_filters_produced() - non_dynamic_filter_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 +1. It would be a good idea to cover that. -- 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]
