Repository navigation
Conversation
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 #2847.
Related: #1055 (the same bug for broadcast hash joins) and #2849 (a similar gap for null-aware anti joins in the static planner).
Rationale for this change
A nested-loop join reads its left (build) side in full and streams its right (probe) side. For
LEFT,FULL, semi, anti and mark joins, it also outputs left-side rows based on whether they matched any right-side row. For example, aLEFT JOINoutputs(left row, NULL)for each left row that matched nothing. DataFusion keeps track of those matches in memory shared by all of the probe side's partitions, and outputs these rows once the last partition is done.Ballista can split the probe side across several tasks, often on different executors. Each task then sees only its share of the right side and decides on its own whether a left row matched. The query returns wrong results, with no error:
EXISTSreturns a row once for each task it matched in, andNOT EXISTSreturns rows that did have a match.For example, with
ordersread from four files and one partition per task:DataFusion returns 4 rows and Ballista returns 13, with both the adaptive and the static planner.
The bug is easy to miss locally. With the default
ballista.scheduler.max_partitions_per_task(unbounded), one task usually takes the whole stage. On a cluster, a stage is split whenever it has more partitions than one executor has free slots.Hash joins are already protected: a broadcast hash join of these types is turned back into a partitioned join, and a null-aware anti join runs its probe side in one task. Nested-loop joins had no such check.
What changes are included in this PR?
Nested-loop joins of these types (the ones
collect_left_broadcast_saferejects) now run as a single task:DistributedExchangeRulealready adds a coalesce under the probe side of a null-aware anti join. It now does the same for these nested-loop joins. The stage that produces the probe side stays parallel; only the join runs as one task.CoalescePartitionsExecunder the join is not enough here. When an aggregate sits above the join, the planner runsEnsureRequirementsover the join's stage again, and that removes the coalesce because the join does not ask for one.Tests:
nested_loop_join.rsrunsLEFT JOIN,FULL JOIN,EXISTS,NOT EXISTS, a mark join (... OR EXISTS) and aGROUP BYover aLEFT JOINunder both planners with one partition per task, and compares the results with DataFusion. Every case fails onmain.LeftAntijoin and leaves anInnerjoin alone.Are there any user-facing changes?
No API or config changes. These joins now return correct results, at the cost of running the join itself in one task.
INNERjoins and the right-side join types are unchanged.TPC-H plan-stability goldens are unchanged: its only nested-loop joins (q11, q22) are
INNER.