adriangb commented on issue #18856: URL: https://github.com/apache/datafusion/issues/18856#issuecomment-5359882807
My pushback on adding a new state is that it's not clear what producers (i.e. `HashJoinExec`) are supposed to do with the information. Let's consider your proposed change in https://github.com/apache/datafusion/pull/24528 ([diff](https://github.com/apache/datafusion/pull/24528/changes#diff-40a284c689424c410dd364bfd40cb8181ce85fa14d6f5cfbaa36eb378d10302b)): `!matches!(filter.discriminant, PushedDown::Unsupported)`. As far as `HashJoinExec` is concerned there are 2 states: `Unsupported` and `Exact`/`Inexact`. > One alternative I considered was artificially pushing filters down to the network nodes by doing a plan rewrite [...] This feels like a smell and I'm not sure if it works in other projects. Could you elaborate on why this seems wrong or wouldn't work? To me it seems like the right thing to do and it should work. My mental model, which may be wrong or outdated, is that we start with a plan like: ```rust HashJoinExec FilterExec [col > 5] DataSourceExec ``` The distributed optimizer runs on this and produces a plan of the form: ```rust HashJoinExec FilterExec [col > 5] NetworkShuffleExec DataSourceExec ``` Now we run the filter pushdown optimizer and get: ```rust HashJoinExec NetworkShuffleExec [ dynamic_filter_propagator ] DataSourceExec [col > 5, dynamic_filter_receiver ] ``` It's up to NetworkShuffleExec to decide that: 1. It will push down the static `col > 5` filter as is 2. It hooks into the dynamic filter updates in it's node and pushes down something that listens for updates. Then you split the plan up: ```rust stage1: HashJoinExec NetworkShuffleExec [ dynamic_filter_propagator ] stage2: DataSourceExec [col > 5, dynamic_filter_receiver ] ``` So `NetworkShuffleExec` is responsible for replying `Yes` to filter pushdown i.e. `Yes this filter will be consumed downstream`. -- 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]
