Skip to content

Commit 31a4ca0

Browse files
adriangbclaude
andauthored
test: characterize correlated NOT IN null-aware behaviour (#25558)
## Which issue does this PR close? - Part of #25336. Closes nothing on its own. ## Rationale for this change Correlated `NOT IN` returns wrong results in several shapes, and a few do not plan at all. The fixes span two layers — the hash join executor and decorrelation — and touch different shapes. Landing the coverage first, pinned to what DataFusion does today, makes each fix's diff show exactly which behaviour it changes instead of burying the flips in a large change. Nothing here changes behaviour. Every expectation is what `main` produces right now, and the wrong ones carry a note and a link to the issue. Concretely, this is wrong today (DuckDB 1.5.2 and PostgreSQL 17.11 both return three rows): ```sql CREATE TABLE oc(id INT, g INT) AS VALUES (1,5),(2,5),(3,0),(4,NULL),(NULL,5),(NULL,0); CREATE TABLE ic(id INT) AS VALUES (1),(NULL); SELECT id, g FROM oc WHERE oc.id NOT IN (SELECT ic.id FROM ic WHERE oc.g > 0); -- returns no rows ``` ## What changes are included in this PR? sqllogictest, in `null_aware_anti_join.slt` and `null_aware_mark_join.slt`: - correlated `NOT IN` with a non-equality correlation, which stays a residual join filter; - a correlation naming only outer columns, so it cannot become an equi-join key; - a constant value expression, with and without a correlation; - a subquery inside the `IN` value, both spellings of the outer `IN`; - `IS NOT NULL` over a subquery predicate, and a comparison between two marks — the contexts that can tell a NULL mark from a FALSE mark; - plan pins for the join the planner picks in each case. Benchmarks: - **Q09**, a correlated non-negated `IN`. It must *not* use a null-aware join. Q01–Q08 all cover the direction where null-awareness is required, so a regression that adds it where it is not needed is invisible to them. Q09 passes today. - correctness canaries on Q05–Q08, each comparing the `NOT IN` result against a reference that does not use `NOT IN`. All four disagree today, so they are pinned to `false`. ## What is the testing strategy for this PR? This PR is tests. Expected results were verified against DuckDB 1.5.2 and PostgreSQL 17.11. The whole suite is green on `main`, including all nine benchmarks. ## Are there any user-facing changes? No. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Adrian Garcia Badaracco <adriangb@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 708617a commit 31a4ca0

9 files changed

Lines changed: 763 additions & 13 deletions

File tree

‎benchmarks/sql_benchmarks/null_aware_join/benchmarks/q05.benchmark‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,22 @@ group null_aware_join
33

44
load sql_benchmarks/null_aware_join/init/load.sql
55

6+
# Correctness canary: the NOT IN result must match a reference count
7+
# that does not use NOT IN. It holds for every NAJ_ROWS / NAJ_LARGE_ROWS.
8+
# As Q04, with NULL outer keys excluded unless the subquery is empty.
9+
assert I
10+
SELECT count(*) = (
11+
SELECT count(*) FROM small_outer o
12+
WHERE o.z <= (SELECT min(z) FROM small_inner)
13+
OR (o.id_n1 IS NOT NULL AND NOT (o.id % 2 = 0 AND (o.id / 2) % 1000 < o.z))
14+
)
15+
FROM small_outer o
16+
WHERE o.id_n1 NOT IN (SELECT i.id_n0 FROM small_inner i WHERE i.z < o.z);
17+
----
18+
# Pinned to today's behaviour, which is incorrect. See
19+
# https://github.com/apache/datafusion/issues/25336 -- the fix flips this.
20+
false
21+
622
expect_plan HashJoinExec
723
expect_plan null_aware
824

‎benchmarks/sql_benchmarks/null_aware_join/benchmarks/q06.benchmark‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,22 @@ group null_aware_join
33

44
load sql_benchmarks/null_aware_join/init/load.sql
55

6+
# Correctness canary: the NOT IN result must match a reference count
7+
# that does not use NOT IN. It holds for every NAJ_ROWS / NAJ_LARGE_ROWS.
8+
# As Q04, with NULL outer keys excluded unless the subquery is empty.
9+
assert I
10+
SELECT count(*) = (
11+
SELECT count(*) FROM small_outer o
12+
WHERE o.z <= (SELECT min(z) FROM small_inner)
13+
OR (o.id_n50 IS NOT NULL AND NOT (o.id % 2 = 0 AND (o.id / 2) % 1000 < o.z))
14+
)
15+
FROM small_outer o
16+
WHERE o.id_n50 NOT IN (SELECT i.id_n0 FROM small_inner i WHERE i.z < o.z);
17+
----
18+
# Pinned to today's behaviour, which is incorrect. See
19+
# https://github.com/apache/datafusion/issues/25336 -- the fix flips this.
20+
false
21+
622
expect_plan HashJoinExec
723
expect_plan null_aware
824

‎benchmarks/sql_benchmarks/null_aware_join/benchmarks/q07.benchmark‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,24 @@ group null_aware_join
33

44
load sql_benchmarks/null_aware_join/init/load.sql
55

6+
# Correctness canary: the NOT IN result must match a reference count
7+
# that does not use NOT IN. It holds for every NAJ_ROWS / NAJ_LARGE_ROWS.
8+
# A row is TRUE when the subquery is empty, or when the subquery holds no NULL
9+
# and the key is not in it. A NULL is in scope when its z is below o.z.
10+
assert I
11+
SELECT count(*) = (
12+
SELECT count(*) FROM small_outer o
13+
WHERE o.z <= (SELECT min(z) FROM small_inner)
14+
OR (o.z <= (SELECT min(z) FROM small_inner WHERE id_n50 IS NULL)
15+
AND NOT (o.id % 2 = 0 AND (o.id / 2) % 1000 < o.z))
16+
)
17+
FROM small_outer o
18+
WHERE o.id_n0 NOT IN (SELECT i.id_n50 FROM small_inner i WHERE i.z < o.z);
19+
----
20+
# Pinned to today's behaviour, which is incorrect. See
21+
# https://github.com/apache/datafusion/issues/25336 -- the fix flips this.
22+
false
23+
624
expect_plan HashJoinExec
725
expect_plan null_aware
826

‎benchmarks/sql_benchmarks/null_aware_join/benchmarks/q08.benchmark‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,30 @@ group null_aware_join
33

44
load sql_benchmarks/null_aware_join/init/load.sql
55

6+
# Correctness canary: the NOT IN result must match a reference count
7+
# that does not use NOT IN. It holds for every NAJ_ROWS / NAJ_LARGE_ROWS.
8+
# A row is TRUE when o.z > 900, when the subquery for its k is empty, or when
9+
# its key is not NULL and not in that subquery.
10+
assert I
11+
SELECT count(*) = (
12+
SELECT count(*)
13+
FROM small_outer o
14+
JOIN (SELECT k, min(z) AS min_z FROM small_inner GROUP BY k) m ON m.k = o.k
15+
WHERE o.z > 900
16+
OR o.z <= m.min_z
17+
OR (o.id_n50 IS NOT NULL
18+
AND NOT (o.id % 2 = 0 AND (o.id / 2) % 16 = o.k AND (o.id / 2) % 1000 < o.z))
19+
)
20+
FROM small_outer o
21+
WHERE o.z > 900
22+
OR o.id_n50 NOT IN (
23+
SELECT i.id_n0 FROM small_inner i WHERE i.k = o.k AND i.z < o.z
24+
);
25+
----
26+
# Pinned to today's behaviour, which is incorrect. See
27+
# https://github.com/apache/datafusion/issues/25336 -- the fix flips this.
28+
false
29+
630
expect_plan HashJoinExec
731

832
run
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
name Q09
2+
group null_aware_join
3+
4+
load sql_benchmarks/null_aware_join/init/load.sql
5+
6+
# Correctness canary: the positive IN result must match a reference count that
7+
# does not use IN. It holds for every NAJ_ROWS. A row is TRUE when o.z > 900 or
8+
# when some in-scope subquery row equals its key; a NULL mark and a FALSE mark
9+
# keep the same rows here, which is what makes the plain mark join correct.
10+
assert I
11+
SELECT count(*) = (
12+
SELECT count(*)
13+
FROM small_outer o
14+
WHERE o.z > 900
15+
OR EXISTS (
16+
SELECT 1 FROM small_inner i
17+
WHERE i.k = o.k AND i.z < o.z AND i.id_n0 = o.id_n50
18+
)
19+
)
20+
FROM small_outer o
21+
WHERE o.z > 900
22+
OR o.id_n50 IN (
23+
SELECT i.id_n0 FROM small_inner i WHERE i.k = o.k AND i.z < o.z
24+
);
25+
----
26+
true
27+
28+
expect_plan HashJoinExec
29+
30+
run
31+
-- Q9: the same correlated shape as Q08, but a NON-negated IN. A Filter keeps a
32+
-- row only when the predicate is TRUE, and AND/OR make TRUE only out of TRUE,
33+
-- so a NULL mark behaves exactly like a FALSE mark and the mark join must NOT
34+
-- be null-aware. Every other query in this suite covers the direction where
35+
-- null-aware handling is required; this one covers the direction where taking
36+
-- it anyway is pure cost. Widening the "needs null-aware" test until this shape
37+
-- is included does not change any result, so only this timing shows it.
38+
SELECT count(*)
39+
FROM small_outer o
40+
WHERE o.z > 900
41+
OR o.id_n50 IN (
42+
SELECT i.id_n0 FROM small_inner i WHERE i.k = o.k AND i.z < o.z
43+
);
44+
45+
cleanup sql_benchmarks/null_aware_join/init/cleanup.sql

‎benchmarks/sql_benchmarks/null_aware_join/null_aware_join.suite‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
description = "Null-aware (NOT IN) hash join benchmarks: uncorrelated, non-equality-correlated, and equality-correlated NOT IN across NULL fractions"
1+
description = "Null-aware (NOT IN) hash join benchmarks: uncorrelated, non-equality-correlated, and equality-correlated NOT IN across NULL fractions, plus the positive IN shape that must not be null-aware"
22

33
query_pattern = "q{QUERY_ID_PADDED}.benchmark"
44

@@ -8,7 +8,7 @@ short = "r"
88
env = "NAJ_ROWS"
99
default = "10000"
1010
values = ["10000", "..."]
11-
help = "Rows per table for the correlated NOT IN queries (Q04-Q08). These evaluate the join filter over candidate build x probe pairs, so their cost grows with the square of this value."
11+
help = "Rows per table for the correlated queries (Q04-Q09). These evaluate the join filter over candidate build x probe pairs, so their cost grows with the square of this value."
1212

1313
[[options]]
1414
name = "large-rows"

0 commit comments

Comments
 (0)