feat: Replace current hash join implementation with Grace hash join [experimental]#3632
Draft
andygrove wants to merge 9 commits intoapache:mainfrom
Draft
feat: Replace current hash join implementation with Grace hash join [experimental]#3632andygrove wants to merge 9 commits intoapache:mainfrom
andygrove wants to merge 9 commits intoapache:mainfrom
Conversation
GHJ supports all join types with either build side, so remove the canBuildShuffledHashJoinLeft/Right checks and the LeftAnti/LeftSemi BuildRight guard (apache#457, apache#2667).
GHJ supports all join types with either build side, but the intermediate ShuffledHashJoinExec node is validated by Spark before CometExecRule replaces it. Use the optimal build side when allowed, otherwise fall back to whichever side Spark permits.
- HashJoin test now matches CometGraceHashJoinExec and checks GHJ metrics (no build_mem_used, adds spill_count) - BroadcastHashJoin test removes build_mem_used assertion since the native side does not report this metric - Remove dead CometHashJoinExec case class (createExec already produces CometGraceHashJoinExec)
Skip build-side hash partitioning when the fast path threshold is set. Instead of always computing hashes and splitting every build batch into N partitions (only to collect them back together for the fast path), buffer the build side directly. When the build fits in memory and is under the threshold, feed it straight to HashJoinExec with zero partitioning overhead. Falls back to the partitioned slow path on memory pressure or when the build exceeds the threshold. Also fix CometConf fastPathThreshold type from intConf to longConf to support values > 2 GB without integer overflow, and remove a duplicate config line in the benchmark TOML. ~4% improvement on both TPC-H and TPC-DS benchmarks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Which issue does this PR close?
Research towards #2545
Follows on from earlier research in #3564
Rationale for this change
Comet's current experimental hash join operator is not suitable for use in production because it has no spilling support and will OOM if the build side is too large. For example, it was not possible to run the TPC-DS benchmarks with hash joins enabled prior to this PR.
This PR replaces it with an experimental Grace hash join operator which does have spilling.
Benchmark Results
What changes are included in this PR?
How are these changes tested?