feat: add dynamic filter store in coordinator - #592
Conversation
Add a registry in the query coordinator for collecting dynamic filter expressions by expression ID. These can be collected during planning once `apply_expressions` becomes available. This store counts received expressions (so we know when we've collected all the filters from workers in a task) and supports the merge() operation which ORs them, following the [RFC](#553). Remaining work - implmement dynamic filter update collection and propagation from worker <-> coordinator
| &self.metrics, | ||
| self.metrics_store.clone(), | ||
| // Dynamic-filter discovery will supply the query's expression IDs here. | ||
| std::iter::empty(), |
There was a problem hiding this comment.
Eventually, I will implement a plan traversal which collects dynamic filters using apply_expressions
jayshrivastava
changed the title
| pub(super) struct DynamicFilterStore { | ||
| expressions: Mutex<HashMap<ExpressionId, Vec<Arc<dyn PhysicalExpr>>>>, | ||
| } |
There was a problem hiding this comment.
It's probably too soon to start speculating about how this will look like.
For example, I'm certain that this is not going to be enough, and that we should be using a Tokio Watch for broadcasting updates.
I recommend to defer this work until we can actually verify that dynamic filters work end to end, otherwise we will be merging code that I'm pretty sure we'll need to heavily change in the future.
One thing we can start doing is building on top of a branch that cherry picks all necessary work upstream, and creating these new extra structs in this project for having dynamic filters working end to end.
There was a problem hiding this comment.
No problem! I'll close this out
2 participants
This PR adds a registry in the query coordinator for collecting dynamic filter expressions by expression ID. These can be collected during planning once
apply_expressionsbecomes available.This store counts received expressions (so we know when we've collected all the filters from workers in a task) and supports the merge() operation which ORs them, following the RFC.
Informs: #532
Remaining work