Repository navigation
Conversation
… where it can gcs_cost_insights_v2 joins object paths to table locations with a broadcast hash join on (bucket, parts[0]) and a filter of 20 conjuncts `isnull(loc.parts[i]) OR loc.parts[i] = obj.parts[i]`. One key holds 11.7k locations, so the join meets 25.7e9 candidate pairs a night. For every pair apply_join_filter_to_indices took both whole `parts` lists into an intermediate batch and evaluated all 40 GetArrayItem and the whole AND tree: 3.6 us per pair on Comet against <= 0.13 us in Spark, 26 task-h instead of 0.9 in one stage. HashJoinStream now evaluates the filter with LazyJoinFilter: - the top-level AND is split and each conjunct is evaluated only over the pairs the earlier ones kept, like Spark's short-circuiting AND; - the largest non-volatile subexpressions that read one input alone (`loc.parts[3]`, `isnull(loc.parts[3])`, `obj.parts[3]`) are lifted out and a pair only copies their results and the bare columns its conjunct reads; - lifted build-side expressions are evaluated per pair until the stream has evaluated as many pairs as there are build rows, then once over the build batch and cached for the rest of the stream; if that fails on some row, they stay per pair, so only rows a pair reaches can raise errors; - without the cache, lifted expressions are evaluated once per run of pairs sharing a row (a probe row with many matches), otherwise per pair over the columns they read, taken once per lookup and filtered with the survivors. Pairs keep their order, so outer/semi/anti/mark handling is unchanged. Filters reading no column, and the symmetric hash join, keep the old path. New bench spark-expr/benches/hash_join_array_filter.rs (Comet ListExtract, the 20-conjunct location filter, batch 8192), base -> this commit: skewed, 12k locations x 200 objects of one key (2.4M pairs): 131 -> 44 ms unique keys (200k pairs, all pass every conjunct): 19.2 -> 17.2 ms Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Cluster validation (datasci-ab playground, em64 12x8, pyspark-comet-env 1.1-13 + this PR, gcs_cost_insights_v2 ds 2026-10-17, count + sum(xxhash64(*)) of attach_best_table_location):
Apps spark-e0396c99bd464f719e5ea6e777988b1c, spark-13eb452dab24448c8994ac055104ffb7, spark-8f2ebaf0ced4486aaf70155db69f4208 (deleted). Local CometJoinSuite: 62 / 0. |
Co-Authored-By: Claude Opus 5.5 (1M context) <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
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?
Closes #. (Joom fork, gcs_cost_insights_v2 on Comet: 4.5x slower than vanilla, mostly in one broadcast hash join)
Rationale for this change
gcs_cost_insights_v2 joins object paths to table locations with a broadcast hash join on
(bucket, parts[0])and a join filter of 20 conjunctsisnull(loc.parts[i]) OR loc.parts[i] = obj.parts[i](40GetArrayItem). One key holds 11.7k of the 19k locations, so a night meets 25.7e9 candidate pairs (inventory) and 10.6e9 (accesses). For every pairapply_join_filter_to_indicestook both wholepartslists into an intermediate batch and evaluated the whole AND tree: 3.6 us per pair against <= 0.13 us in Spark; stage 209 26.1 task-h vs 0.9 vanilla (joom-1.1-12, ds 2026-10-17). Upstream apache/datafusion main has the same code path.What changes are included in this PR?
HashJoinStreamevaluates its filter with the newhash_join/lazy_filter.rsLazyJoinFilter(vendored physical-plan):loc.parts[3],isnull(loc.parts[3]),obj.parts[3]) are lifted out; a pair copies only their results and the bare columns its conjunct reads, never the lists;Pair order is preserved, so outer/semi/anti/mark handling after the filter is unchanged. A filter that reads no column keeps the old path; the symmetric hash join is untouched.
How are these changes tested?
lazy_filter.rstests: the kept pairs equalapply_join_filter_to_indices(old path) exactly, in order, for 9 filters (the 5-level parts filter, mixed conjuncts, OR at the top, CASE, nullable conjuncts, NULL literal conjunct, probe-only, build-only + probe-only, strict cast) over sorted and shuffled pairs, before and after the build cache; errors only for pairs the filter reaches (cast error of an unpaired build row is not raised, of a paired one it is); lifted expressions evaluated per row, not per pair; HashJoinExec equals NestedLoopJoinExec overl.k = r.k AND filterfor all 10 join types x batch sizes 7/64/8192 x 9 filters.spark-expr/benches/hash_join_array_filter.rs(CometListExtract, the 20-conjunct filter, batch 8192), base -> this PR: skewed 12k locations x 200 objects of one key (2.4M pairs) 131 -> 44 ms (55 -> 18 ns/pair); unique keys (200k pairs passing every conjunct) 19.2 -> 17.2 ms.CometJoinSuite"Broadcast HashJoin with a join filter over array elements of both sides" (INNER, LEFT, LEFT SEMI, LEFT ANTI, RIGHT vs Spark).-D warningsclean (workspace; vendored crate except the pre-existingneedless_pass_by_valuein sorts/sort/wide_payload.rs).🤖 Generated with Claude Code