[SPARK-55848][SQL][TESTS] Add regression tests for SPJ partial clustering dedup by naveenp2708 · Pull Request #54714 · apache/spark

naveenp2708 · 2026-03-10T05:06:59Z

What changes were proposed in this pull request?

Test-only PR. Adds regression tests for SPARK-55848 (SPJ partial clustering produces incorrect results for post-join dedup operations).

Three tests added to KeyGroupedPartitioningSuite:

SPARK-55848: dropDuplicates after SPJ with partial clustering
SPARK-55848: Window dedup after SPJ with partial clustering
SPARK-55848: checkpointed scan with partial clustering and dedup

Why are the changes needed?

The fix was merged via #54330, but regression tests for the correctness issue (SPARK-55848 / #54378) were not included. These tests ensure the issue does not regress.

Does this PR introduce any user-facing change?

No. Test-only change.

How was this patch tested?

All 73 tests in KeyGroupedPartitioningSuite pass.

Was this patch authored or co-authored using generative AI tooling?

No.

…dup correctness Add regression tests verifying that dropDuplicates, Window dedup, and checkpointed scans produce correct results when SPJ partial clustering is active. These tests cover the correctness issue fixed by SPARK-55848. Closesapache#54378

naveenp2708 · 2026-03-10T05:07:53Z

@peter-toth Per your suggestion — tests-only PR for master. Branch-4.1 PR with fix + tests coming next.

peter-toth · 2026-03-10T08:51:54Z

Seems like check didn't run on this PR yet, most likely because you haven't enabled Github Actions in your forked repo.
Can you please check the "Pull request" section at https://spark.apache.org/contributing.html? Or if you click on the pending check that also shows you how to enable/retrigger the necessary workflows.

peter-toth · 2026-03-10T15:27:45Z

+ |FROM testcat.ns.$items i
+ |JOIN testcat.ns.$purchases p ON i.id = p.item_id
+ |""".stripMargin)
+ checkAnswer(df, Seq(Row(1), Row(2), Row(3)))


Can you please check if we have the expected GroupPartitionExecs in the plan? I believe one should be under the join to align partitions to the other, partially clustered side of the join; and one should be above the join to group partitions for the aggregate.
Also please assert that we don't have any exchanges in the plan.

peter-toth · 2026-03-10T15:40:35Z

+ // Checkpoint the items scan, then join with purchases under partial clustering,
+ // and finally dedup. The checkpointed scan preserves the KeyedPartitioning
+ // with non-grouped keys; the dedup must still produce correct distinct ids.
+ val itemsDf = spark.read.table(s"testcat.ns.$items").checkpoint()


I mean in this test we should checkpoint the partially clustered result of a join and put an aggregate on top of that checkpoint. The aggregate should require a GroupPartitionsExec on master or a shuffle on previous branches (before the refactor).

naveenp2708 · 2026-03-10T16:54:52Z

@peter-toth Added GroupPartitionsExec assertions to all 3 tests:

2 GroupPartitionsExec under the join (one per child to align partitions)
1 GroupPartitionsExec above the join (for aggregate/window grouping)
Total of 3 GroupPartitionsExec per test plan
enabled the workflows on my fork. CI should run now.

naveenp2708 · 2026-03-10T23:37:54Z

@peter-toth Enabled the workflows on my fork — "Build" and "Report test results" both passed.

peter-toth · 2026-03-11T12:29:58Z

@naveenp2708, can you please take a look at #54751 (comment) and adjust the test case there and here as well? Actually, probably we should agree on the fix in #54751 first and then update this PR.

szehon-ho

Thanks for the tests!

szehon-ho · 2026-03-12T23:37:57Z

+ }
+ }
+
+ test("SPARK-55848: Window dedup after SPJ with partial clustering should produce " +


nit: 'should produce correct results' is redundant :)

szehon-ho · 2026-03-12T23:42:01Z

+ // SPJ. The WINDOW operator requires ClusteredDistribution on i.id; with partial
+ // clustering the plan must insert the right exchange/group so that the window
+ // produces exactly one row per id.
+ val df = sql(


should we use 'selectWithMergeJoinHint' for consistency?

naveenp2708 · 2026-03-13T02:28:39Z

@szehon-ho Fixed both — shortened test names and switched to selectWithMergeJoinHint. Thanks for the review!

naveenp2708 · 2026-03-13T22:02:34Z

@peter-toth Makes sense — PR #54751 now has approvals from you and @szehon-ho. Once it's merged, I'll update this PR to align the tests. Already addressed szehon-ho's nits (shortened test names + selectWithMergeJoinHint).

peter-toth · 2026-03-16T18:03:45Z

@naveenp2708 , I merged #54751, can you please update this PR? Also, let's add [TESTS] tag to this one.

naveenp2708 · 2026-03-17T03:53:57Z

@peter-toth Updated title with [TESTS] tag. Tests aligned with merged #54751 — checkpointed test checkpoints join result, AQE disabled, GroupPartitionsExec assertions, shortened names, selectWithMergeJoinHint. Will open backport PRs for branch-4.0 and branch-3.5 next.

peter-toth · 2026-03-17T10:40:48Z

@naveenp2708 , can you please double check the PR title? It looks incomplete and I don't see [TESTS] there; and also retrigger the checks (the failure seems unrelated)?

naveenp2708 · 2026-03-17T22:44:06Z

@peter-toth Fixed the title to include [TESTS] tag. Retriggered the checks.

peter-toth · 2026-03-18T17:29:59Z

@peter-toth Fixed the title to include [TESTS] tag. Retriggered the checks.

Thank you @naveenp2708. Could you please retrigger the tests again?

naveenp2708 · 2026-03-18T17:43:51Z

@peter-toth Retriggered. Thanks for the approval!

naveenp2708 · 2026-03-20T04:26:08Z

@peter-toth @dongjoon-hyun Resolved the merge conflict and retriggered CI.

peter-toth · 2026-03-20T12:07:34Z

Thank you @naveenp2708.

Merged to master (4.2.0).

…ring dedup ### What changes were proposed in this pull request? Test-only PR. Adds regression tests for SPARK-55848 (SPJ partial clustering produces incorrect results for post-join dedup operations). Three tests added to KeyGroupedPartitioningSuite: 1. SPARK-55848: dropDuplicates after SPJ with partial clustering 2. SPARK-55848: Window dedup after SPJ with partial clustering 3. SPARK-55848: checkpointed scan with partial clustering and dedup ### Why are the changes needed? The fix was merged via apache#54330, but regression tests for the correctness issue (SPARK-55848 / apache#54378) were not included. These tests ensure the issue does not regress. ### Does this PR introduce any user-facing change? No. Test-only change. ### How was this patch tested? All 73 tests in KeyGroupedPartitioningSuite pass. ### Was this patch authored or co-authored using generative AI tooling? No. Closesapache#54714 from naveenp2708/spark-55848-tests-master. Authored-by: Naveen Kumar Puppala <naveenp2708@gmail.com> Signed-off-by: Peter Toth <peter.toth@gmail.com>

peter-toth reviewed Mar 10, 2026
View reviewed changes

naveenp2708 force-pushed the spark-55848-tests-master branch from fb9cadc to 1b1b1cbCompare March 10, 2026 16:33

szehon-ho reviewed Mar 12, 2026
View reviewed changes

naveenp2708 force-pushed the spark-55848-tests-master branch from 1b1b1cb to ad6cf20Compare March 13, 2026 02:28

trigger CI
2442dea

naveenp2708 force-pushed the spark-55848-tests-master branch from ad6cf20 to 2442deaCompare March 17, 2026 03:52

retrigger CI
efb0b6c

naveenp2708 changed the title ~~[SPARK-55848][SQL] Add regression tests for SPJ partial clustering de…~~[SPARK-55848][SQL][TESTS] Add regression tests for SPJ partial clustering dedupMar 17, 2026

retrigger CI
3ef8a80

peter-toth approved these changes Mar 18, 2026
View reviewed changes

retrigger CI
249121d

Resolve merge conflict in imports
eeebe9d

peter-toth closed this in c67c02aMar 20, 2026

Uh oh!

Conversation

naveenp2708 commented Mar 10, 2026

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

naveenp2708 commented Mar 10, 2026

Uh oh!

peter-toth commented Mar 10, 2026

Uh oh!

peter-tothMar 10, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peter-tothMar 10, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

naveenp2708 commented Mar 10, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

naveenp2708 commented Mar 10, 2026

Uh oh!

peter-toth commented Mar 11, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

szehon-ho left a comment

Choose a reason for hiding this comment

Uh oh!

szehon-hoMar 12, 2026

Choose a reason for hiding this comment

Uh oh!

szehon-hoMar 12, 2026

Choose a reason for hiding this comment

Uh oh!

naveenp2708 commented Mar 13, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

naveenp2708 commented Mar 13, 2026

Uh oh!

peter-toth commented Mar 16, 2026

Uh oh!

naveenp2708 commented Mar 17, 2026

Uh oh!

peter-toth commented Mar 17, 2026

Uh oh!

naveenp2708 commented Mar 17, 2026

Uh oh!

peter-toth commented Mar 18, 2026

Uh oh!

naveenp2708 commented Mar 18, 2026

Uh oh!

naveenp2708 commented Mar 20, 2026

Uh oh!

peter-toth commented Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

peter-tothMar 10, 2026•
edited
Loading

peter-tothMar 10, 2026•
edited
Loading

naveenp2708 commented Mar 10, 2026•
edited
Loading

peter-toth commented Mar 11, 2026•
edited
Loading

naveenp2708 commented Mar 13, 2026•
edited
Loading