Commit 9fba8b3
committed
Simplify wait_complete function (apache#19937)
## Which issue does this PR close?
## Rationale for this change
The current v52 signature `pub async fn wait_complete(self: &Arc<Self>)`
(introduced in apache#19546) is a bit unergonomic. The method requires
`&Arc<DynamicFilterPhysicalExpr>`, but when working with `Arc<dyn
PhysicalExpr>`, downcasting only gives you `&DynamicFilterPhysicalExpr`.
Since you can't convert `&DynamicFilterPhysicalExpr` to
`Arc<DynamicFilterPhysicalExpr>`, the method becomes impossible to call.
The `&Arc<Self>` param was used to check` is_used()` via Arc strong
count, but this was overly defensive.
## What changes are included in this PR?
- Changed `DynamicFilterPhysicalExpr::wait_complete` signature from `pub
async fn wait_complete(self: &Arc<Self>)` to `pub async fn
wait_complete(&self)`.
- Removed the `is_used()` check from `wait_complete()` - this method,
like `wait_update()`, should only be called on filters that have
consumers. If the caller doesn't know whether the filter has consumers,
they should call `is_used()` first to avoid waiting indefinitely. This
approach avoids complex signatures and dependencies between the APIs
methods.
## Are these changes tested?
Yes, existing tests cover this functionality, I removed the "mock"
consumer from `test_hash_join_marks_filter_complete_empty_build_side`
and `test_hash_join_marks_filter_complete` since the fix in
apache#19734 makes is_used check the
outer struct `strong_count` as well.
## Are there any user-facing changes?
The signature of `wait_complete` changed.
(cherry picked from commit bef1368)1 parent 97453f2 commit 9fba8b3
2 files changed
Lines changed: 10 additions & 17 deletions
File tree
- datafusion
- physical-expr/src/expressions
- physical-plan/src/joins/hash_join
Lines changed: 10 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
272 | 272 | | |
273 | 273 | | |
274 | 274 | | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
275 | 279 | | |
276 | 280 | | |
277 | 281 | | |
| |||
283 | 287 | | |
284 | 288 | | |
285 | 289 | | |
286 | | - | |
287 | | - | |
| 290 | + | |
288 | 291 | | |
289 | 292 | | |
290 | 293 | | |
291 | 294 | | |
292 | | - | |
293 | | - | |
294 | | - | |
295 | | - | |
296 | | - | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
297 | 300 | | |
298 | 301 | | |
299 | 302 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4633 | 4633 | | |
4634 | 4634 | | |
4635 | 4635 | | |
4636 | | - | |
4637 | | - | |
4638 | | - | |
4639 | | - | |
4640 | | - | |
4641 | 4636 | | |
4642 | 4637 | | |
4643 | 4638 | | |
| |||
4686 | 4681 | | |
4687 | 4682 | | |
4688 | 4683 | | |
4689 | | - | |
4690 | | - | |
4691 | | - | |
4692 | | - | |
4693 | | - | |
4694 | 4684 | | |
4695 | 4685 | | |
4696 | 4686 | | |
| |||
0 commit comments