You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Is your feature request related to a problem or challenge?
Disclaimer: I'm writing this from the perspective of datafusion-distributed, but I imagine that this problem is common to other projects (ballista, comet) that distribute datafusion queries. I am not familiar with comet or ballista, so I would appreciate any feedback from folks who work on those projects.
Datafusion natively preserves dynamic filter producer<->consumer relationships across network boundaries only when you serialize the producer (HashJoinExec) and the consumer (DataSourceExec) in serialization run. There's tests which assert that this works.
Distributed projects (ballista, comet, datafusion-distributed) should be responsible for implementing dynamic filter update routing and propagation during query execution
Vanilla datafusion should provide a way to find and extract dynamic filters from ExecutionPlan to allow a project like datafusion-distributed to implement (1). The minimal required features are
(a) identify which dynamic filters are producers and consumers in an ExecutionPlan tree
(b) get access to all &DynamicFilterPhysicalExpr in an ExecutionPlan (as long as one is holding a reference to a DynamicFilterPhysicalExpr, they can read and write updates to it)
(c) allow easy extension for custom ExecutionPlan implementations
The proposed dynamic_filter_exprs() API is simpler to implement, making the breaking change less annoying
Still no use case in vanilla datafusion
Pros:
Does exactly what we want
Cons:
Breaking change. It must be implemented by every ExecutionPlan. We cannot default to vec![] because ExecutionPlan nodes may fail to declare their dynamic filters.
Very specific. We usually treat dynamic filters as any other filter expression. However, this method cuts through the abstraction and brings them to the forefront.
Option 2 - Store Dynamic Filters in the Session (SessionState)
We can try updating the SessionState to store which plan nodes are producers and consumers of dynamic filters. This can be populated during the FilterPushdown optimization rule using the existing API:
We could make it the distributed project's responsibility to mutate / update the FilterPushdownReport if it changes any ExecutionPlans, but that would still rely on pointer comparisons.
It would be nice if vanilla datafusion committed to a minimal feature (ie. "DynamicFilterPhysicalExpr can be discovered from ExecutionPlan") with minimal tests support use cases like datafusion-distributed, ballista, and comet.
Describe alternatives you've considered
We could downcast ExecutionPlan to concrete types and try to discover dynamic filters like that, however this method is less extensible (how would we support custom ExecutionPlan implementations?) and it isn't guaranteed to work because not every concrete type will make their dynamic filters pub.
As of writing, these public methods on some execution plan nodes exist, byt they were only added to support serialization and will likely be removed by #23494.
Is your feature request related to a problem or challenge?
Disclaimer: I'm writing this from the perspective of datafusion-distributed, but I imagine that this problem is common to other projects (ballista, comet) that distribute datafusion queries. I am not familiar with comet or ballista, so I would appreciate any feedback from folks who work on those projects.
More context in the (WIP) Dynamic Filtering in DataFusion-Distributed RFC
Background
Datafusion natively preserves dynamic filter producer<->consumer relationships across network boundaries only when you serialize the producer (
HashJoinExec) and the consumer (DataSourceExec) in serialization run. There's tests which assert that this works.The problem is that datafusion doesn't support the case where you split up the consumer from the producer:
Describe the solution you'd like
Proposal:
ExecutionPlanto allow a project like datafusion-distributed to implement (1). The minimal required features are(a) identify which dynamic filters are producers and consumers in an
ExecutionPlantree(b) get access to all
&DynamicFilterPhysicalExprin anExecutionPlan(as long as one is holding a reference to aDynamicFilterPhysicalExpr, they can read and write updates to it)(c) allow easy extension for custom
ExecutionPlanimplementationsOption 1 - New
ExecutionPlanAPINotes
ExecutionPlan::apply_expressions()(#20337)" #22437 because implementing it was complex and there was no use case in vanilla datafusion to assert correctnessdynamic_filter_exprs()API is simpler to implement, making the breaking change less annoyingPros:
Cons:
ExecutionPlan. We cannot default tovec![]becauseExecutionPlannodes may fail to declare their dynamic filters.Option 2 - Store Dynamic Filters in the
Session(SessionState)We can try updating the
SessionStateto store which plan nodes are producers and consumers of dynamic filters. This can be populated during theFilterPushdownoptimization rule using the existing API:The
PhysicalOptimizerContextonly stores a statistics registry today. We would need to extend it to store dynamic filters:Here's a rough sketch of what we would store in the
Session.Pros
Cons
Arc<dyn PhysicalExpr>references may grow stale.datafusion-distributedcertainly modifies theExecutionPlantree and could invalidate these pointers.FilterPushdownPhase::Postsays it should be run at the end of the optimization process since any changes to the plan may break the dynamic filter’s references.FilterPushdownReportif it changes anyExecutionPlans, but that would still rely on pointer comparisons.Open questions
Describe alternatives you've considered
We could downcast
ExecutionPlanto concrete types and try to discover dynamic filters like that, however this method is less extensible (how would we support customExecutionPlanimplementations?) and it isn't guaranteed to work because not every concrete type will make their dynamic filterspub.As of writing, these public methods on some execution plan nodes exist, byt they were only added to support serialization and will likely be removed by #23494.
Additional context
No response