bench: pwmj left semi/anti join - #24160
Conversation
github-actions
Bot
added
sqllogictest
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #24160 +/- ##
==========================================
+ Coverage 81.02% 81.05% +0.02%
==========================================
Files 1106 1106
Lines 380718 382025 +1307
Branches 380718 382025 +1307
==========================================
+ Hits 308489 309651 +1162
- Misses 54001 54091 +90
- Partials 18228 18283 +55 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@kumarUjjawal build successful. PTAL |
| BenchmarkId::new(format!("pwmj_{jt}_{regime}"), right_rows), | ||
| |b| { | ||
| b.iter(|| { | ||
| let (left, right) = build_inputs(); |
There was a problem hiding this comment.
build_inputs() is timed, including generating 40,000 values and allocating arrays and batches. This can dominate the ~0.5 ms PWMJ result. Could we prebuild the batches and use iter_batched for fresh plans outside the timed section?
| // Selectivity is set by how far the right key range sits above the left range. | ||
| // - "high": right keys mostly above left keys -> most left rows match (Semi large) | ||
| // - "low": right keys mostly below left keys -> few left rows match (Anti large) | ||
| let regimes: [(&str, i32); 2] = [("sel_high", key_span), ("sel_low", -key_span)]; |
There was a problem hiding this comment.
These offsets produce exactly all-match and no-match cases, not “mostly” and “few.” Could we label them as all/none and add an overlapping-range case to measure partial suffix marking?
| Arc::new(Column::new("key", 0)), | ||
| ); | ||
| Arc::new( | ||
| PiecewiseMergeJoinExec::try_new( |
There was a problem hiding this comment.
Could we benchmark this through the SQL/physical planner instead of constructing PiecewiseMergeJoinExec directly? The current benchmark will panics on because existence joins are unsupported until #23870.
github-actions
Bot
added
core
kumarUjjawal
left a comment
kumarUjjawal
left a comment
There was a problem hiding this comment.
Hi @SubhamSinghal Thanks for iterating.
This became a little tricky that's why we had to do more follow up. If you have any ideas you can share as-well.
| ] { | ||
| let ctx = create_context(right_offset, pwmj, &s); | ||
| let name = format!("{arm}_{label}_{regime}"); | ||
| assert_plan_contains( |
There was a problem hiding this comment.
This still panics on this PR’s current head because enabling PWMJ selects NestedLoopJoinExec until #23870, while this assertion requires PiecewiseMergeJoin. To use this revision as the pre-change benchmark baseline, could we name this arm pwmj_enabled and accept either NLJ or PWMJ, while keeping the disabled arm pinned to NLJ? #23870’s tests can assert that the enabled plan switches to PWMJ.
|
@comphead this is benchmark PR |
comphead
left a comment
comphead
left a comment
There was a problem hiding this comment.
Thanks @SubhamSinghal for the PR.
Please update the PR description accordingly
4 participants
Which issue does this close?
Benchmark companion to #23870 (LeftSemi / LeftAnti support for
PiecewiseMergeJoinExec), part of EPIC #17427. Closes no issue on its own.Rationale for this change
#23870 routes existence subqueries with an inequality correlation (
WHERE EXISTS (SELECT 1 FROM rhs WHERE lhs.key < rhs.key)) toPiecewiseMergeJoinExecinstead ofNestedLoopJoinExec. That claim needs a benchmark that can be run on either side of the change, so this PR adds the benchmark separately from the operator work — it is bench code only, no functional change.What changes are included in this PR?
datafusion/core/benches/pwmj_semi_anti_sql.rs(+ itsCargo.tomlentry), a Criterion benchmark with two arms over 20k × 20kInt32rows,target_partitions=1:pwmj_enabled— flag onnlj— flag off, which can only planNestedLoopJoinExecAxes: join type (
EXISTS→ Semi,NOT EXISTS→ Anti) × match regime (all_match100%,no_match0%,half_match~50%, so the buffered side is only partially marked and scan depth varies per streamed row).Are these changes tested?
Are there any user-facing changes?
No. Benchmark code only.