[SPARK-59438][SQL][TESTS] Add planner-stage test coverage for ASOF JOIN sort-merge operator - #58742
Draft
LukaZdravic wants to merge 1 commit into
Draft
[SPARK-59438][SQL][TESTS] Add planner-stage test coverage for ASOF JOIN sort-merge operator#58742LukaZdravic wants to merge 1 commit into
LukaZdravic wants to merge 1 commit into
Conversation
…IN sort-merge operator Adds two planner-stage tests for the ASOF JOIN sort-merge path, both mirroring existing sibling-join test patterns: - `PlannerSuite`: a no-equi-key `AsOfJoin` requires a single partition on both sides (the `AllTuples` branch of `SortMergeAsOfJoinExec`'s `requiredChildDistribution`), verified by running `EnsureRequirements` and asserting a single-partition shuffle on each side. Follows the existing `SortMergeJoinExec` distribution tests (SPARK-24495 / SPARK-27485). - `SortMergeAsOfJoinSuite`: a null-safe (`<=>`) equi-key in `ON` is routed to the residual condition rather than treated as an equi-key, so null keys on both sides do match -- the counterpart to the existing `EqualTo` case where null keys never match. Mirrors `InnerJoinSuite`'s "inner join, null safe". Test-only change; no production code is modified.
There was a problem hiding this comment.
🟢 Approval recommended
Test-only changes with no unresolved issues.
Pull request overview
Adds test-only planner and execution coverage for ASOF JOIN behavior.
Changes:
- Tests single-partition requirements for no-equi-key joins.
- Tests null-safe conditions matching null keys.
File summaries
| File | Summary |
|---|---|
sql/core/src/test/scala/org/apache/spark/sql/SortMergeAsOfJoinSuite.scala |
Adds null-safe ASOF JOIN coverage. |
sql/core/src/test/scala/org/apache/spark/sql/execution/PlannerSuite.scala |
Adds distribution requirement coverage. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
What changes were proposed in this pull request?
This is a test-only change that adds planner-stage (physical-planning / strategy) coverage for the
ASOF JOINsort-merge operator:PlannerSuite— a new test constructs a no-equi-keySortMergeAsOfJoinExecand runsEnsureRequirements, asserting that both sides are shuffled to a single partition. This exercises theAllTuplesbranch of the operator'srequiredChildDistribution(used when the join has no equi-keys), following the existingSortMergeJoinExecdistribution tests in the same suite (SPARK-24495 / SPARK-27485).SortMergeAsOfJoinSuite— a new test checks that a null-safe equi-key (<=>) inONis routed to the residual condition rather than treated as an equi-key, so rows whose key isNULLon both sides do match. This is the counterpart to the existingEqualTocase where null keys never match, and mirrorsInnerJoinSuite's "inner join, null safe" test.No production code is changed.
Why are the changes needed?
The
AsOfJoinSelectionstrategy andSortMergeAsOfJoinExecoperator were exercised end-to-end for result correctness, but two planner-stage behaviors had no test: (1) the operator's customrequiredChildDistributionthat forces a single partition when there are no equi-keys, and (2) the strategy's handling ofEqualNullSafeinON. Sibling join operators cover the analogous behaviors; this bringsASOF JOINup to the same bar.Does this PR introduce any user-facing change?
No.
How was this patch tested?
New unit tests. Ran
PlannerSuiteandSortMergeAsOfJoinSuitelocally (via Maven); all tests pass.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code