Skip to content

bench: SQL benchmark suite for null-aware (NOT IN) joins - #25386

Merged
adriangb merged 4 commits into
apache:mainfrom
pydantic:claude/relaxed-meitner-ng4ng4
Sep 17, 2026
Merged

adriangb merged 4 commits into
apache:mainfrom
pydantic:claude/relaxed-meitner-ng4ng4

Conversation

@adriangb

@adriangb adriangb commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

A NOT IN subquery becomes a null-aware join. An outer row that finds no match is TRUE only when neither side has a NULL in scope. If a NULL is in scope, the result is UNKNOWN.

This decision is cheap for an uncorrelated NOT IN. For a correlated NOT IN, the correlation predicate stays behind as a join filter. The join must then evaluate that filter for each candidate (build row x probe row) pair, to find which rows the NULLs reach. A non-equality correlation gives no equality key, so there is no scope key to reduce the number of pairs. The cost then grows with the NULL count multiplied by the size of the opposite table.

No benchmark measured this shape, so there was no way to see the cost, or to tell a change from noise. Review on #25339 asked for this benchmark.

These are the measured results for #25339. Each number is the median of 60 iterations, taken as 6 interleaved rounds of 10 iterations on an Apple M4 Pro in release mode. The two sides are the base commit of #25339 and its head commit, each with this suite applied, so the comparison isolates the change in that PR.

Query Shape base #25339
Q01 uncorrelated, non-nullable keys 17.9 ms 17.7 ms 0.99x
Q02 uncorrelated, 1% NULL subquery side 14.9 ms 14.9 ms 1.00x
Q03 uncorrelated, 50% NULL outer side 14.6 ms 14.6 ms 1.00x
Q04 correlated, nullable keys, no NULL present 0.9 ms 0.9 ms 0.96x
Q05 correlated, 1% NULL outer side 0.9 ms 2.9 ms 3.1x
Q06 correlated, 50% NULL outer side 0.9 ms 96.2 ms 109x
Q07 correlated, 50% NULL subquery side 0.8 ms 94.4 ms 112x
Q08 as Q06, with an equality correlation 1.1 ms 15.1 ms 14x

Q01 to Q04 are the comparable rows, and they show no change. The base gives wrong results for Q05 to Q08, which is the bug that #25339 corrects. Thus the base numbers for those four rows are the time to calculate an incorrect result. They show the cost of correct results, not a regression. These are the results at the default sizes. DuckDB agrees with the "correct" column.

Query correct (#25339) base
Q05 7460 7450
Q06 5010 5000
Q07 10 0
Q08 5530 10000

Q06 and Q07 are the rows that the review of #25339 asked about. They also give the baseline to measure any later optimization of that path against. Q08 has the same NULL fraction as Q06 and is 6 times cheaper, which is the value of the equality correlation.

What changes are included in this PR?

A null_aware_join SQL benchmark suite. There are no Rust changes. The runner finds suites in benchmarks/sql_benchmarks/, and the load SQL makes each table from range(), so there is no data generation step.

  • Q01 to Q03 are uncorrelated NOT IN at different NULL fractions. Their cost is linear with the table size. They are the regression guard for the plain null-aware path.
  • Q04 is the correlated shape with nullable keys that hold no NULL. It separates the baseline cost of the shape from the per-pair filter work.
  • Q05 to Q07 are the same correlation at 1% and 50% NULL on each side. This is where that work becomes visible.
  • Q08 has the same NULL fraction as Q06, but adds an equality correlation. The candidate pairs then come from a hash lookup. The difference between Q06 and Q08 shows the value of the scope key.

Both table sizes are knobs. NAJ_ROWS (default 10000) sets the size for the correlated queries, whose cost grows with its square. NAJ_LARGE_ROWS (default 1000000) sets the size for the uncorrelated queries.

./bench.sh run null_aware_join

# One query, with more rows for the correlated shape
NAJ_ROWS=20000 ./bench.sh run null_aware_join 6

This PR also adds the suite to bench.sh (including all) and documents it in benchmarks/README.md and benchmarks/sql_benchmarks/README.md.

There is one Rust change: the Debug output of HashJoinExec now includes null_aware. The suite's expect_plan directive matches that output, so Q02 to Q07 can require null_aware: true. Before this change, expect_plan HashJoinExec also passed for a plain anti join.

What is the testing strategy for this PR?

This PR adds benchmarks, so it adds no new tests. The existing checked_in_suites_cover_benchmark_directories test in benchmarks/src/sql_benchmark_suite.rs covers suite discovery, and it passes with the new directory.

Each query has these checks:

I ran all eight queries on this branch at the default sizes and at -r 1500 -l 101. As a counterfactual check on main, the Q05 to Q08 asserts fail, and null_aware: true fails on Q01 and Q08.

Each query also runs on main as written. Q08 uses the mark join form on purpose. The plain WHERE ... NOT IN form with an equality correlation does not plan on main, and a query that runs on only one branch cannot compare two branches.

Are there any user-facing changes?

No. This PR changes benchmarks and documentation only. It does not change library code.

🤖 Generated with Claude Code

A NOT IN subquery plans as a null-aware join, where an outer row that finds
no match is TRUE only if neither side has a NULL in scope and UNKNOWN
otherwise. Deciding that is cheap for an uncorrelated NOT IN, but a
correlated one leaves its correlation predicate behind as a join filter, and
the join has to evaluate that filter per candidate (build row x probe row)
pair to work out which rows the NULLs actually reach. Without an equality
correlation there is no scope key to narrow those pairs, so the cost grows
with the NULL count times the opposite table's size.

Nothing measured that shape, so add a null_aware_join suite covering it:

- Q01-Q03 uncorrelated NOT IN across NULL fractions, linear in the table
  size, as the regression guard for the plain null-aware path.
- Q04 the correlated shape with nullable keys that hold no NULL, so the
  zero-NULL baseline is separated from the per-pair filter work.
- Q05-Q07 the same correlation at 1% and 50% NULL on each side, which is
  where that work shows up.
- Q08 the same NULL fraction as Q06 but with an equality correlation, so the
  candidate pairs come from a hash lookup instead; the gap between the two is
  what the scope key buys.

All tables are built inline from range(), so there is no data step. Sizes are
knobs: NAJ_ROWS for the correlated queries, NAJ_LARGE_ROWS for the rest.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019tS7mXDfq2faC9Xe6EXtZB
@codecov-commenter

codecov-commenter commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.32%. Comparing base (6ff484a) to head (e6e7989).
⚠️ Report is 23 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25386      +/-   ##
==========================================
+ Coverage   81.93%   82.32%   +0.38%     
==========================================
  Files        1136     1137       +1     
  Lines      428859   431825    +2966     
  Branches   428859   431825    +2966     
==========================================
+ Hits       351376   355488    +4112     
+ Misses      56462    54848    -1614     
- Partials    21021    21489     +468     

☔ 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.

@kosiew kosiew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adriangb,

Thanks for adding this benchmark coverage. The new suite does a nice job covering both correlated and uncorrelated NOT IN shapes. I found one small integration issue with the default all benchmark path, but it is non-blocking.

Comment thread benchmarks/bench.sh
mkdir -p "${RESULTS_DIR}"
mkdir -p "${DATA_DIR}"
case "$BENCHMARK" in
all)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small integration nit: all(default) is documented as running all benchmarks, but the all branch does not currently call run_null_aware_join, so the new regression benchmark gets silently skipped in the default aggregate run. Could we add run_null_aware_join here? If the default runtime makes it unsuitable for all, it would be good to explicitly document that instead.

@jayzhan211 jayzhan211 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @adriangb , some non blocking suggestions


load sql_benchmarks/null_aware_join/init/load.sql

expect_plan HashJoinExec

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expect_plan matches the {:#?} output, and HashJoinExec's Debug impl doesn't print null_aware, so this guard also passes for a plain (non-null-aware) join — Q08 on main is exactly that. Worth pinning, since the whole suite is about that flag:

             .field("mode", &self.mode)
+            .field("null_aware", &self.null_aware)
             .field("metrics", &self.metrics)

then on Q02–Q08:

-expect_plan HashJoinExec
+expect_plan null_aware: true

Fine as a follow-up

-- same query at 50%.
SELECT count(*)
FROM small_outer o
WHERE o.id_n1 NOT IN (SELECT i.id_n0 FROM small_inner i WHERE i.z < o.z);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On main this suite measures wrong answers for Q05–Q08, so the "base" column in the table is the cost of skipping the work, not a slower/faster comparison. Checked against DuckDB at default sizes:

Query correct main
Q05 7460 7450
Q06 5010 5000
Q07 10 0
Q08 5530 10000

parquet_row_filter_skip already uses assert as a correctness canary for the same reason. Please add one per query (in #25339 if they need to stay red on main until it lands), and note in this PR's description that the base numbers for Q05–Q08 come from incorrect results. Example for Q07:

 expect_plan HashJoinExec

+assert I
+SELECT count(*)
+FROM small_outer o
+WHERE o.id_n0 NOT IN (SELECT i.id_n50 FROM small_inner i WHERE i.z < o.z);
+----
+10
+
 run

Fine as a follow-up. We can revisit the result later to figure out whether there's a bug in DataFusion, or whether the expected result legitimately differs from DuckDB.

adriangb and others added 3 commits September 17, 2026 09:06
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
HashJoinExec's Debug output did not include null_aware, so
`expect_plan HashJoinExec` also passed for a plain anti join. Add the
field to Debug and require `null_aware: true` on Q02-Q07.

Q01 has non-nullable keys, so it is not null-aware. Q08 plans as a plain
mark join on main until apache#25339 lands, so it keeps only
the HashJoinExec check here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each assert compares the NOT IN count with a reference count that does
not use NOT IN, so it holds at every NAJ_ROWS / NAJ_LARGE_ROWS value.
Q05-Q08 give wrong results on main (apache#25336), so their
asserts go in apache#25339 together with the fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Sep 17, 2026
@adriangb

Copy link
Copy Markdown
Contributor Author

Addressed reviews, thanks for the approvals folks!

@adriangb
adriangb added this pull request to the merge queue Sep 17, 2026
@jayzhan211

Copy link
Copy Markdown
Contributor

It seems Q1-Q4 are fixed, should we go with Q5-Q8?

The queries the PR's 3x–112x numbers come from have no correctness canary, and on main they return wrong results, so the "base" column is timing an incorrect fast path. x NOT IN (<empty>) is TRUE, but main drops every outer row whose scope is empty (o.z = 0) once a NULL exists anywhere, and Q08 returns all rows. Measured on main at defaults (NOT IN count / reference count):

Query NOT IN reference
Q05 7450 7460
Q06 5000 5010
Q07 0 10
Q08 10000 5530

These asserts fail on main, so land them in #25339 (where they should pass) rather than here, and note in this PR's table that base Q05–Q08 are wrong-result timings:

-- q05 / q06 (swap id_n1 for id_n50 in q06)
assert I
SELECT count(*) = (
  SELECT count(*) FROM small_outer o
  WHERE o.z = 0
     OR (o.id_n1 IS NOT NULL AND NOT (o.id % 2 = 0 AND (o.id / 2) % 1000 < o.z))
)
FROM small_outer o
WHERE o.id_n1 NOT IN (SELECT i.id_n0 FROM small_inner i WHERE i.z < o.z);
----
true

-- q07: inner v = 0 is NULL and has z = 0, so it is in scope for every o.z > 0
assert I
SELECT count(*) = (SELECT count(*) FROM small_outer WHERE z = 0)
FROM small_outer o
WHERE o.id_n0 NOT IN (SELECT i.id_n50 FROM small_inner i WHERE i.z < o.z);
----
true

-- q08
assert I
SELECT count(*) = (
  SELECT count(*) FROM small_outer o
  WHERE o.z > 900
     OR NOT EXISTS (SELECT 1 FROM small_inner i WHERE i.k = o.k AND i.z < o.z)
     OR (o.id_n50 IS NOT NULL
         AND NOT (o.id % 2 = 0 AND (o.id / 2) % 16 = o.k AND (o.id / 2) % 1000 < o.z))
)
FROM small_outer o
WHERE o.z > 900
   OR o.id_n50 NOT IN (
        SELECT i.id_n0 FROM small_inner i WHERE i.k = o.k AND i.z < o.z
      );
----
true

@jayzhan211

Copy link
Copy Markdown
Contributor

Never mind that could be done follow-up 😆

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 17, 2026
@adriangb
adriangb added this pull request to the merge queue Sep 17, 2026
Merged via the queue into apache:main with commit c4f72ba Sep 17, 2026
42 checks passed
@adriangb
adriangb deleted the claude/relaxed-meitner-ng4ng4 branch September 17, 2026 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants