feat: extend SLT tests for hash join vs PWMJ, hash join vs SMJ

[comphead]

Which issue does this PR close?

• Closes Create a combined correctness join SLT tests #24641 .

Rationale for this change

PiecewiseMergeJoinExec and SortMergeJoinExec must return the same results as
the mature join implementations they can be swapped for, across every batch
boundary. Today that equivalence is checked ad hoc. Using the # configMatrix:
directive from #24493, one .slt file can assert it directly: run the same
queries once per join implementation and once per batch size, and require
identical output.

What changes are included in this PR?

What is the testing strategy for this PR?

Are there any user-facing changes?

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.57%. Comparing base (791b32e) to head (7cc8d2d).
⚠️ Report is 27 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24805      +/-   ##
==========================================
+ Coverage   81.50%   81.57%   +0.07%     
==========================================
  Files        1123     1123              
  Lines      404760   406610    +1850     
  Branches   404760   406610    +1850     
==========================================
+ Hits       329896   331712    +1816     
+ Misses      55552    55458      -94     
- Partials    19312    19440     +128     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[viirya]

Nice to see configMatrix from #24493 get its first real use — this is exactly the kind of equivalence checking that's been done ad hoc until now, and the two files read well.

I ran both files on top of current main and they pass. I also checked that the matrix isn't accidentally vacuous, since that's the usual failure mode for this shape of test — with these small fixtures I half expected the SMJ side to collapse into CollectLeft HashJoin on both arms. It doesn't: prefer_hash_join=false really does plan SortMergeJoinExec and true plans HashJoinExec, and on the PWMJ side enable_piecewise_merge_join flips between PiecewiseMergeJoin (both Inner and LeftSemi) and NestedLoopJoinExec. So the differential is real on both files.

What I liked most is that the cases are written against the implementation rather than being generic smoke tests. The ej_dup_l.v = {5,5,3,3,1} fixture with a deciding streamed key of 3, and the comment about binary search needing to return the first index of a run of equal keys, lands right on the boundary that the suffix-watermark encoding from #24579 depends on. Same for splitting all-NULL buffered from all-NULL streamed, and for sweeping batch_size=1. The "Rules for a matrix file" note in both headers is a good idea too — it should stop someone quietly breaking the matrix later by adding an EXPLAIN or an in-file SET.

Two things, one substantive and one a nit.

Mark joins in the SMJ file. The file covers Inner/Left/Right/Full and LeftSemi/LeftAnti/RightSemi/RightAnti, but I don't see a mark join. Was that a deliberate scope decision? Asking because SMJ does seem to implement LeftMark/RightMark (there's dedicated handling in sort_merge_join/bitwise_stream.rs), the existing sort_merge_join.slt does cover mark joins, and when I tried ... WHERE l.k = 3 OR EXISTS (SELECT 1 FROM smj_r r WHERE r.k = l.k) under this file's own settings it planned as SortMergeJoinExec: join_type=LeftMark. If that's right, mark joins would be the one existence type not getting the batch-size sweep here — and being the type that emits one row per build row with a boolean, it's arguably the one where batch boundaries are most interesting. Would you consider adding it, or is there a reason it's better left in sort_merge_join.slt?

The RightSemi claim in the PWMJ header. The header says RightSemi/RightAnti/Mark "stay on NestedLoopJoin in both combinations". That's true of the queries in the file — I checked, the explicit RIGHT SEMI JOIN / RIGHT ANTI JOIN shapes do stay on NLJ, so the tests and expected values are fine, and the "(explicit syntax)" qualifier is doing real work. But the same semantics written as a correlated EXISTS gets its correlation flipped by the planner and came out as PiecewiseMergeJoin ... join_type=LeftSemi for me. Since the sentence reads like a property of the join type rather than of the SQL shape, could it be worth making that distinction explicit? Otherwise someone may later reason from that line and conclude a RightSemi-flavoured query can't reach PWMJ.

Neither of these blocks merging from my side — the file passes and the coverage it adds is real.

[kumarUjjawal]

Thank you @comphead

This is nice! I left one comment for your consideration.

[kumarUjjawal]

batch_size=1 does not split this VALUES input into one-row batches. This matrix does not cover the stated per-batch extreme-key path or the cross-batch watermark path. Can we build the streamed fixture from a source that emits several batches, such as generate_series and add a plan or metric assertion.

[comphead]

mark joins make sense, totally slipped my mind, checking them

batch_size=1 does not split this VALUES input into one-row batches. This matrix does not cover the stated per-batch extreme-key path or the cross-batch watermark path. Can we build the streamed fixture from a source that emits several batches, such as generate_series and add a plan or metric assertion.

sounds good to me.

[viirya]

Re-reviewed the update. Both of my earlier points and @kumarUjjawal's are handled well — and the batch-realism fix is better than what I'd have suggested.

Mark joins (my earlier point): addressed, and then some. mark_join_matrix.slt doesn't just add coverage, it exploits the fact that the two arms plan different orientations. I confirmed that: prefer_hash_join=false gives SortMergeJoinExec: join_type=LeftMark and true gives HashJoinExec: mode=CollectLeft, join_type=RightMark, inputs swapped. So the matrix cross-checks LeftMark against RightMark rather than one operator against itself, which is a stronger assertion than I had in mind. The NOT IN pair is a good touch too — keeping a NULL-free mk_r_nn alongside mk_r isolates the mark join's non-null-aware negation from top-level three-valued logic, and the comment says so explicitly.

The batch_size question: the split into two *_batches.slt plan proofs is the right call. What I like is that it doesn't apply one blanket answer — it identifies that the two operators need different justifications. For SMJ, SortExec re-batches to batch_size, so equal-key runs really do span batches even from VALUES, and the file proves it with input_batches=16, input_rows=16 on a 9x7 fixture. For PWMJ the streamed side isn't re-batched, so a real multi-batch source was needed, and the plan shows LazyMemoryExec: batch_generators=[range: start=3, end=8, batch_size=2] feeding a PiecewiseMergeJoin rather than a fallback. The header conceding "a VALUES source would not show batch_size in the plan" is the honest framing. I checked the SMJ arithmetic independently and 26 is right, so the count assertions aren't just recording current behaviour.

Also re-ran all five files on current main: they pass.

One small thing that regressed. In v1 the PWMJ header read RightSemi/RightAnti (explicit syntax) and Mark (OR EXISTS) stay on NestedLoopJoin; the condensed v2 header now says just RightSemi/RightAnti/Mark stay on NLJ (line ~23, repeated ~171). The tests are still correct — the explicit RIGHT SEMI JOIN / RIGHT ANTI JOIN queries in the file do stay on NLJ — but the qualifier that made the sentence true is gone. Written as a correlated EXISTS, the planner flips the correlation and I get PiecewiseMergeJoin ... join_type=LeftSemi, so "RightSemi doesn't reach PWMJ" holds for the SQL shape rather than the join type. Since the rewrite dropped that distinction, could the (explicit syntax) note come back? It's the one line someone might later reason from and be misled by.

That's a comment-only nit and doesn't need to gate this. Nice work on the plan proofs.