chore: convert PartialHashAggregateStream and FinalHashAggregateStream to async generators and cleanup - #24017
chore: convert PartialHashAggregateStream and FinalHashAggregateStream to async generators and cleanup#24017rluvaton wants to merge 4 commits into
PartialHashAggregateStream and FinalHashAggregateStream to async generators and cleanup#24017Conversation
|
run benchmarks |
|
run benchmark tpcds |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing move-final-hash-stream (8a49479) to 541caab (merge-base) diff using: tpcds File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing move-final-hash-stream (8a49479) to 541caab (merge-base) diff using: clickbench_partitioned File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing move-final-hash-stream (8a49479) to 541caab (merge-base) diff using: tpch File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing move-final-hash-stream (8a49479) to 541caab (merge-base) diff using: tpcds File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #24017 +/- ##
========================================
Coverage 80.84% 80.85%
========================================
Files 1096 1099 +3
Lines 373936 374146 +210
Branches 373936 374146 +210
========================================
+ Hits 302313 302515 +202
+ Misses 53584 53574 -10
- Partials 18039 18057 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Could we delay those cleanup PRs after #22710 is functionally complete? Now we're in a inconsistent state, and I think it's better to fully remove the legacy implementation sooner, and next we can continue cleaning things up. I estimate it's ~3 PRs away from that, I'll update in the EPIC issue when it's ready. |
Which issue does this PR close?
Related to:
Rationale for this change
Cleanup the code and remove state
What changes are included in this PR?
changed
PartialHashAggregateStreamandFinalHashAggregateStreamto async generator and remove unneeded code due to thatThe first commit in this PR is
FinalHashAggregateStreamthe second commit is
PartialHashAggregateStreamAre these changes tested?
existing tests
Are there any user-facing changes?
nope