Reuse hash values during Aggregate(partial) -> Repartition -> Aggregate(final)#23720
Draft
gabotechs wants to merge 5 commits into
Draft
Reuse hash values during Aggregate(partial) -> Repartition -> Aggregate(final)#23720gabotechs wants to merge 5 commits into
gabotechs wants to merge 5 commits into
Conversation
github-actions
Bot
added
physical-expr
Contributor
Author
|
run benchmarks tpch10 |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing gabotechs/reuse-hashes (45fa16b) to d0304b3 (merge-base) diff using: tpch10 File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpch10 — base (merge-base)
tpch10 — branch
File an issue against this benchmark runner |
gabotechs
changed the title
Contributor
Author
|
run benchmarks clickbench_partitioned |
|
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 |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing gabotechs/reuse-hashes (45fa16b) to d0304b3 (merge-base) diff using: clickbench_partitioned 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 |
Contributor
Author
|
run benchmarks tpcds |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing gabotechs/reuse-hashes (45fa16b) to d0304b3 (merge-base) diff using: tpcds 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 |
gabotechs
force-pushed
the
gabotechs/reuse-hashes
branch
from
July 20, 2026 12:00
45fa16b to
1158083
Compare
gabotechs
force-pushed
the
gabotechs/reuse-hashes
branch
from
July 20, 2026 13:24
1158083 to
1c84f94
Compare
Contributor
|
At a highlevel, this change sounds good. It would be interesting to see if the partial aggregation skipping behavior can be disabled with no impact. I think this should do it. Since the hashes are re-used the partial aggregation should be less expensive. |
Contributor
Author
|
Yeah, I can imagine how you'd want to enable this based on the hashing expressions. For example, maybe hashing on a simple I still want to see if we can reach a situation where this enabled 100% of the times is worth it, that would simplify things a bit. |
Contributor
I think for primitives I found it can be a quite slower to store the hash rather than regenerate it inside a loop as hashing is just is a shift / multiply / xor (potentially SIMD) vs a potential cache miss / extra cache usage. For (long) strings it can of course be very beneficial. |
Contributor
Agree, what is worse: the internal hash column will be repartitioned and coalesced together with other normal columns, and
Seems strange that q33 and q34 not get improvement... |
Contributor
Author
|
run benchmarks tpch10 tpcds clickbench_partitioned |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing gabotechs/reuse-hashes (7d64c1d) to d0304b3 (merge-base) diff using: tpch10 File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing gabotechs/reuse-hashes (7d64c1d) to d0304b3 (merge-base) diff using: tpcds File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing gabotechs/reuse-hashes (7d64c1d) to d0304b3 (merge-base) diff using: clickbench_partitioned File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagetpch10 — base (merge-base)
tpch10 — 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 |
gabotechs
force-pushed
the
gabotechs/reuse-hashes
branch
from
July 21, 2026 09:58
7d64c1d to
cbfb9cb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Still in draft 🙈, expect heavy changes
Which issue does this PR close?
Rationale for this change
During partial + final aggregations, in order to accumulate + repartition data cross partitions, a hash is computed over the grouping expressions.
This hash is computed 3 times:
And depending on what's getting hashed, it can be an expensive operation (e.g., hashing arbitrary length strings).
This PR introduces a mechanism that allows reusing the hashes across different operators, by propagating them under an internal column in the schema:
What changes are included in this PR?
Introduces to core structs that aim to set the foundations of hash reuse across operators:
ExpressionHasher: handles hashing columns based on expressions, maintaining an internal buffer for reusing allocations, stamping the hashes into internal columns so that they can cross operator boundaries, and retrieving those hashes from the internal columns if present.GroupHashTracker: during groupings, it keeps track of the per-group hashes so that operators that aggregate can emit them as an additional internal column for reuse purposes.These two structs are used across all the aggregation implementations in the codebase for reusing hashes.
Additionally, some metrics for tracking how many hashes where computed VS how many hashes where reused are also added to operators that need to deal with hashes
Are these changes tested?
Yes, by existing and new tests.
Are there any user-facing changes?
No, this should be transparent to users.