PushdownFilter optimizations #21668
Conversation
8db87e3 to
7aee0a0
Compare
7aee0a0 to
6ce2a70
Compare
|
@joroKr21 If the direction is agreed by the reviewer you should combine the pr, I will be happy to help. |
6ce2a70 to
740cf2c
Compare
|
run benchmark sql_planner |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing opt/pushdown-filter (740cf2c) to 3aefba7 (merge-base) diff File an issue against this benchmark runner |
|
show benchmark queue |
|
Hi @kumarUjjawal, you asked to view the benchmark queue (#21668 (comment)).
File an issue against this benchmark runner |
|
run benchmark push_down_filter |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing opt/pushdown-filter (740cf2c) to 3aefba7 (merge-base) diff File an issue against this benchmark runner |
|
🤖 Criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
@joroKr21 The benchmarks are slower, can you investigate this. |
|
The benchmark from the issue is |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing opt/pushdown-filter (740cf2c) to 3aefba7 (merge-base) diff File an issue against this benchmark runner |
|
Benchmark for this request hit the 7200s job deadline before finishing. Benchmarks requested: Kubernetes messageFile an issue against this benchmark runner |
740cf2c to
af81018
Compare
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
|
This is the relevant benchmark: |
|
run benchmark sql_planner_extended |
|
@joroKr21 the sql extended timed out last time, i have ran with filter let's see |
|
run benchmark sql_planner |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing opt/pushdown-filter (af81018) to d59bc72 (merge-base) diff File an issue against this benchmark runner |
|
Me and Claude found some other issue here (returning |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing opt/pushdown-filter (af81018) to d59bc72 (merge-base) diff File an issue against this benchmark runner |
|
🤖 Criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
We need to run with 10 samples, it took almost an hour for me locally on main |
|
show benchmark queue |
|
Hi @kumarUjjawal, you asked to view the benchmark queue (#21668 (comment)).
File an issue against this benchmark runner |
|
@Dandandan these benchmarks have taking too long to run, is this the right way? |
|
Benchmark for this request hit the 7200s job deadline before finishing. Benchmarks requested: Kubernetes messageFile an issue against this benchmark runner |
af81018 to
ac411f0
Compare
|
@Dandandan I ported your tests here - some were already passing, but I needed to make some changes |
4585f33 to
23c9b82
Compare
|
run benchmark sql_planner |
Not sure |
|
🤖 Criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing opt/pushdown-filter (23c9b82) to bb86364 (merge-base) diff File an issue against this benchmark runner |
- Add a hidden `Filter::new` constructor that skips type-checking - Less allocations, more modification of mutable plan nodes - Less cloning, use references when possible
23c9b82 to
c249d99
Compare
|
🤖 Criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Tests ported from apache#21964
c249d99 to
370bb45
Compare
Which issue does this PR close?
try_uncheckedmore so I would adopt thatRationale for this change
What changes are included in this PR?
Filter::new_uncheckedconstructor that skips type-checkingAre these changes tested?
Relying on existing tests mostly, added a few more tests.
Are there any user-facing changes?
make_filteris removed, probably wasn't meant to be a public function.