Conversation
github-actions
Bot
added
optimizer
adriangb
force-pushed
the
in-exists-subquery-projection
branch
from
September 15, 2026 19:17
9e94b59 to
04f3d7d
Compare
adriangb
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #25338 +/- ##
==========================================
+ Coverage 82.36% 82.41% +0.04%
==========================================
Files 1137 1138 +1
Lines 432937 435575 +2638
Branches 432937 435575 +2638
==========================================
+ Hits 356589 358977 +2388
- Misses 54821 54858 +37
- Partials 21527 21740 +213 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@kosiew since you reviewed #24972 would you mind taking a look here? cc @neilconway since you were involved in #21363 |
Contributor
Author
|
run benchmarks |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing in-exists-subquery-projection (04f3d7d) to 22651d2 (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing in-exists-subquery-projection (04f3d7d) to 22651d2 (merge-base) diff Run configurationrun benchmark tpcdsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing in-exists-subquery-projection (04f3d7d) to 22651d2 (merge-base) diff Run configurationrun benchmark tpchResults will be posted here when complete File an issue against this benchmark runner |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Expression-level nullability can be missed, causing projected IN to return false instead of UNKNOWN.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Optimizes projected IN/NOT IN subqueries by using a single null-aware mark join when possible.
Changes:
- Tracks whether mark joins preserve three-valued logic.
- Retains three-join fallback for residual predicates.
- Adds plan and NULL-semantics regression tests.
File summaries
| File | Description |
|---|---|
datafusion/optimizer/src/decorrelate_predicate_subquery.rs |
Implements single-mark-join optimization. |
datafusion/sqllogictest/test_files/subquery_projection.slt |
Tests plans and NULL semantics. |
datafusion/sqllogictest/test_files/projection_pushdown.slt |
Updates alias-collision regression coverage. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing in-exists-subquery-projection (04f3d7d) to 22651d2 (merge-base) diff Run configurationrun benchmark tpchCPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing in-exists-subquery-projection (04f3d7d) to 22651d2 (merge-base) diff Run configurationrun benchmark tpcdsCPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing in-exists-subquery-projection (04f3d7d) to 22651d2 (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Contributor
Author
|
run benchmark clickbench_partitioned |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing 22651d2 (22651d2) to 22651d2 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
changed:
ref: "22651d24cc8196f3206e09a36a437eda9e766b87"Results will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing 22651d2 (22651d2) to 22651d2 (merge-base) diff Run configurationrun benchmark clickbench_partitioned
changed:
ref: "22651d24cc8196f3206e09a36a437eda9e766b87"CPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
Contributor
Author
|
run benchmark clickbench_partitioned |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing in-exists-subquery-projection (04f3d7d) to 22651d2 (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
Contributor
Author
|
I opened #25346 to add benchmarks |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing in-exists-subquery-projection (04f3d7d) to 22651d2 (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
kosiew
requested changes
Sep 16, 2026
kosiew
left a comment
kosiew
left a comment
Contributor
There was a problem hiding this comment.
Thanks for working on this. Reusing the exact hashable LeftMark join for projected IN / NOT IN is a nice improvement, and the added NULL-semantics coverage is helpful.
I found one blocking issue in the correlated NOT IN path. The expression-level nullability change can now make a LeftAnti join null-aware when there is more than one hash key, but physical planning only supports a single key for null-aware LeftAnti joins. I left a repro and suggested adding an execution regression test below.
I also left one non-blocking suggestion for the empty-subquery boundary on the new single-mark-join path.
| && join_keys_may_be_null(&join_filter, left.schema(), sub_query_alias.schema())?; | ||
| // Additionally, if no join key can be NULL on either side, we don't need | ||
| // null-aware semantics because NULLs cannot exist in the keys. | ||
| let null_aware = if join_type == JoinType::LeftAnti && in_predicate_opt.is_some() { |
Contributor
There was a problem hiding this comment.
I think this introduces a planning failure for correlated NOT IN when the nullable expression key is combined with another correlated equality key.
For example, SELECT id FROM o WHERE NULLIF(id, 1) NOT IN (SELECT id FROM r WHERE r.grp = o.grp) with non-nullable o(id, grp) and r(id, grp) now makes the LeftAnti join null-aware because NULLIF(id, 1) is nullable. The correlation adds grp as a second hash key, so physical planning rejects the resulting join with null_aware LeftAnti joins only support single column join key, got 2 columns.
This looks newly reachable through the expression-level nullability change. Could we preserve the correct correlated NOT IN semantics using a supported fallback or plan shape here? It would also be good to add this query as an execution regression test and assert that only the SQL-true rows are returned.
Contributor
Author
There was a problem hiding this comment.
Thanks, confirmed. The query failed with got 2 columns. It plans again in fe408e3: a LeftAnti join with more than one key keeps the column test from main.
This does not give the correct result for your NULLIF example. The join has two keys and is not null-aware, so the k = 1 row stays, the same as on main. The test in subquery_projection.slt records this, with a comment that links apache/datafusion#25347. A correct plan needs a null-aware LeftAnti join with more than one key. apache/datafusion#25339 (approved) adds that. When it merges, I will remove this special case and update the expected output to the correct rows. I did not want to add a second fallback plan here, because apache/datafusion#25339 is the fix for this.
| 05)----ProjectionExec: expr=[CAST(id@0 AS Int64) as r3.id] | ||
| 06)------DataSourceExec: partitions=1, partition_sizes=[1] | ||
|
|
||
| # `NULLIF(id, 1)` is NULL for `id = 1`, and `r3` has no NULL, so the answer is |
Contributor
There was a problem hiding this comment.
Could we also add an empty-subquery case for the new single null-aware mark path using a typed nullable key expression, for example NULLIF(id, 1)?
In particular, it would be useful to assert that the NULL-key row produces false, not NULL, when the subquery is empty, and that the plan still uses a single mark join. The existing top-level NULL IN (empty) test covers the SQL result semantics, but it takes the legacy three-join path, so it does not protect this boundary of the new optimization.
Contributor
|
Thanks @adriangb , here is a suggestion: Correlated The For a correlated CREATE TABLE t1(k INT NOT NULL, s VARCHAR NOT NULL) AS VALUES (1, 'a');
CREATE TABLE t2(k INT NOT NULL, s VARCHAR NOT NULL) AS VALUES (1, 'B');
SELECT * FROM t1 WHERE upper(t1.s) NOT IN (SELECT t2.s FROM t2 WHERE t2.k = t1.k);
-- main: plans a plain LeftAnti and returns (1, 'a')
-- this PR: expected "null_aware LeftAnti joins only support single column join key"Nullable columns already fail like this on main (#25347), but this change extends the failure to common function keys over non-nullable data. It also pins uncorrelated Suggest limiting the expression-level check on the let null_aware = if join_type == JoinType::LeftAnti && in_predicate_opt.is_some() {
let (equijoin_keys, residual_filter) = split_eq_and_noneq_join_predicate(
join_filter.clone(),
left.schema(),
sub_query_alias.schema(),
)?;
- join_keys_may_be_null(
- &equijoin_keys,
- residual_filter.as_ref(),
- left.schema(),
- sub_query_alias.schema(),
- )?
+ if equijoin_keys.len() == 1 {
+ join_keys_may_be_null(
+ &equijoin_keys,
+ residual_filter.as_ref(),
+ left.schema(),
+ sub_query_alias.schema(),
+ )?
+ } else {
+ // Null-aware LeftAnti supports a single key only (#25347); keep the
+ // previous column-based test so correlated NOT IN still plans.
+ join_filter_columns_may_be_null(&join_filter, left.schema(), sub_query_alias.schema())?
+ }
} else {Please also add an slt case with the correlated query above. |
Contributor
Author
|
Thanks @jayzhan211, good catch. I applied your suggestion in fe408e3. |
Contributor
Author
|
@jayzhan211 @kosiew could we merge the benchmarks in #25346 before this change so we can look at perf numbers? |
Contributor
Author
|
run benchmarks |
Contributor
Author
|
run benchmark projection_subquery |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing in-exists-subquery-projection (3af8737) to 64871d9 (merge-base) diff Run configurationrun benchmark clickbench_partitionedResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing in-exists-subquery-projection (3af8737) to 64871d9 (merge-base) diff Run configurationrun benchmark tpcdsResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing in-exists-subquery-projection (3af8737) to 64871d9 (merge-base) diff Run configurationrun benchmark tpchResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing in-exists-subquery-projection (3af8737) to 64871d9 (merge-base) diff Run configurationrun benchmark tpchCPU Details (lscpu)Details
Resource Usagetpch — base (merge-base)
tpch — branch
File an issue against this benchmark runner |
|
🤖 Benchmark running (GKE) | trigger CPU Details (lscpu)Comparing in-exists-subquery-projection (3af8737) to 64871d9 (merge-base) diff Run configurationrun benchmark projection_subqueryResults will be posted here when complete File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing in-exists-subquery-projection (3af8737) to 64871d9 (merge-base) diff Run configurationrun benchmark tpcdsCPU Details (lscpu)Details
Resource Usagetpcds — base (merge-base)
tpcds — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing in-exists-subquery-projection (3af8737) to 64871d9 (merge-base) diff Run configurationrun benchmark clickbench_partitionedCPU Details (lscpu)Details
Resource Usageclickbench_partitioned — base (merge-base)
clickbench_partitioned — branch
File an issue against this benchmark runner |
|
🤖 Benchmark completed (GKE) | trigger Instance: Comparing in-exists-subquery-projection (3af8737) to 64871d9 (merge-base) diff Run configurationrun benchmark projection_subqueryCPU Details (lscpu)Details
Resource Usageprojection_subquery — base (merge-base)
projection_subquery — branch
File an issue against this benchmark runner |
Contributor
Author
|
Benchmark summary for
q06 and q07 are the controls. q07 keeps the three-join materialization, which this PR does not change, and #25336 covers it. The other suites show no change:
The counts are from the min table. The four tpcds queries that move are q22 (1.05x slower), q43 (1.08x), q45 (1.05x) and q72 (1.11x faster), and the total is lower on the branch. The same comparison one run earlier gave 99 of 99 "no change", so these are the noise of this runner. An absolute reference against DuckDB 1.5.2 is now in the description, under "DuckDB comparison". The short form: |
jayzhan211
reviewed
Sep 19, 2026
jayzhan211
left a comment
jayzhan211
left a comment
Contributor
There was a problem hiding this comment.
Here is another possible issue
Correlation that repeats the IN predicate → wrong NULLs (regression vs base, which returns FALSE). remove_duplicated_filter (decorrelate.rs:189) drops i.k = o.k, the scope key is lost, and the join is planned as an uncorrelated null-aware LeftMark; the raw mark is now exposed. The result can never be UNKNOWN here since every in-scope row has y = x.
Repro:
CREATE TABLE o(id INT, k INT) AS VALUES (1,1),(2,1),(NULL,1),(2,2),(NULL,3),(5,NULL),(9,9);
CREATE TABLE i(id INT, k INT) AS VALUES (1,1),(NULL,2),(7,1),(3,NULL);
-- k=3, k=9, k=NULL: expected false, PR gives NULL
SELECT o.id, o.k, o.k IN (SELECT i.k FROM i WHERE i.k = o.k) AS m FROM o ORDER BY k, id;
-- (NULL,1), (2,2): expected false, PR gives NULL
SELECT o.id, o.k, o.id IN (SELECT i.id FROM i WHERE i.id = o.id AND i.k = o.k) AS m FROM o ORDER BY k, id;
-- same root cause, wrong on main too: expected 3 rows, got 0
SELECT o.id, o.k FROM o WHERE o.k NOT IN (SELECT i.k FROM i WHERE i.k = o.k) ORDER BY k, id;Fix (both repro queries then return the base results; please add them to subquery_projection.slt):
--- a/datafusion/optimizer/src/decorrelate.rs
+++ b/datafusion/optimizer/src/decorrelate.rs
@@ pub struct PullUpCorrelatedExpr {
pub in_predicate_opt: Option<Expr>,
+ /// The subquery repeats the `IN` predicate as a correlation
+ /// (`x IN (SELECT y .. WHERE y = x)`), so the result is never UNKNOWN.
+ pub in_predicate_is_correlation: bool,
@@ fn f_up
+ let num_filters = join_filters.len();
join_filters = remove_duplicated_filter(join_filters, in_predicate)?;
+ self.in_predicate_is_correlation |= join_filters.len() < num_filters;
--- a/datafusion/optimizer/src/decorrelate_predicate_subquery.rs
+++ b/datafusion/optimizer/src/decorrelate_predicate_subquery.rs
@@ fn build_join
let null_aware = match hashable_only_split {
+ Some(_) if pull_up.in_predicate_is_correlation => false,
Some((equijoin_keys, residual_filter)) => join_keys_may_be_null(
@@
- let null_aware = if join_type == JoinType::LeftAnti && in_predicate_opt.is_some() {
+ let null_aware = if join_type == JoinType::LeftAnti
+ && in_predicate_opt.is_some()
+ && !pull_up.in_predicate_is_correlation
+ {Also initialize in_predicate_is_correlation: false in PullUpCorrelatedExpr::new
haohuaijin
pushed a commit
to haohuaijin/arrow-datafusion
that referenced
this pull request
Sep 19, 2026
## Which issue does this PR close? - This PR closes no issue. It is related to apache#25341 and it prepares the A/B measurement for apache#25338. ## Rationale for this change An `IN`, `NOT IN` or `EXISTS` subquery in the `SELECT` list is much slower than the same subquery in a `WHERE` clause. A projected `IN` must return `NULL`, and not `false`, when there is no match and the inner side holds a `NULL`. The decorrelation therefore turns one projected `IN` into more than one mark join. A mark join with no hashable join predicate runs as a nested-loop join, so the cost grows with the outer row count times the inner row count. There is no benchmark suite that measures this shape today. The benchmark bot compares a pull request against its merge base. It can only report a change if the suite is present on both sides. The suite must therefore land on `main` first, before the fix is measured with it. ## What changes are included in this PR? - A new declarative SQL benchmark suite, `projection_subquery`, with seven queries. Each query is one shape: - `q01_in_bare`: bare uncorrelated `IN`. - `q02_in_coalesce`: the same `IN` wrapped in `COALESCE`, which is the common way to use the result. - `q03_in_correlated_eq`: `IN` correlated on an equality. - `q04_in_two_columns`: two independent `IN` subqueries in one `SELECT` list. - `q05_not_in_bare`: `NOT IN`. - `q06_exists_correlated`: correlated `EXISTS`, which has no `NULL` result and needs only one mark join. - `q07_in_correlated_residual`: `IN` correlated on `<`. There is no equality to hash on, so this query keeps the nested-loop plan. It is the control: its time must not change when the hashable shapes get faster. - Two small integer tables built inline by the suite load SQL, so the suite needs no data step. Every 97th inner key is `NULL`, which keeps three-valued logic in play. Set `PSQ_ROWS` to change the row count in each table. The default is 30000. - An aggregate over the projected boolean in every query, so the result is two numbers and the measured cost is the plan and not the size of the output. - Checked-in result files, so `--result-mode validate` also proves that a change to the decorrelation still returns the same rows. The counts hold for the default `PSQ_ROWS`. - `bench.sh` wiring: `./benchmarks/bench.sh data projection_subquery` reports that there is no external data, and `./benchmarks/bench.sh run projection_subquery` runs the suite. - Documentation in `benchmarks/README.md` and in the suite table of `benchmarks/sql_benchmarks/README.md`. ## What is the testing strategy for this PR? This PR adds no product code, so there are no unit tests. It was verified by running the suite. `./benchmarks/bench.sh data projection_subquery` prints the no-data message. `./benchmarks/bench.sh run projection_subquery` builds `cargo bench --bench sql` and completes with a time for each of the seven queries. `--result-mode validate` passes on `main` and on the branch of apache#25338, which shows that both sides return the same rows. Median of five iterations at the default `PSQ_ROWS` of 30000, on one machine: | Query | main | With apache#25338 | | --- | --- | --- | | `q01_in_bare` | 341 ms | 0.8 ms | | `q02_in_coalesce` | 690 ms | 1.1 ms | | `q03_in_correlated_eq` | 2.6 ms | 0.9 ms | | `q04_in_two_columns` | 527 ms | 1.2 ms | | `q05_not_in_bare` | 349 ms | 0.6 ms | | `q06_exists_correlated` | 0.6 ms | 0.4 ms | | `q07_in_correlated_residual` (control) | 66 ms | 63 ms | The control query is unchanged, as expected, because it has no equality to hash on. ## Are there any user-facing changes? No. This PR adds benchmarks only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…`IN` predicate
`x IN (SELECT y FROM .. WHERE y = x)` writes the `IN` equality a second
time, as a correlated filter. `remove_duplicated_filter` drops that filter,
because `build_join` adds the same equality back as the join filter. The
scope of the subquery is then gone, and the join looked uncorrelated and
null-aware, so a miss gave UNKNOWN.
The result of this shape is never UNKNOWN: every row the subquery keeps for
an outer row satisfies `y = x`, so `y` is not NULL there and the subquery
result is either empty or `{x}`. `PullUpCorrelatedExpr` now reports the
shape, and the mark join and the anti join stay plain for it.
A node above the dropped filter can still put a NULL into the value column,
and then the result is UNKNOWN again. An outer join, a union and a grouping
set do this, so the flag is cleared when the pull up passes one of them.
`f_up` walks the subquery bottom up, so every such node above the filter is
reached after it.
This also fixes apache#25480, which is
the same root cause in a `WHERE ... NOT IN` clause.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The new `subquery_projection.slt` section holds the three queries from the review: the `IN` whose correlation is the `IN` predicate, the same with a second correlation, and the `WHERE ... NOT IN` form. It also pins the plan, which no longer says `null_aware`, and the `ROLLUP` shape, which still does because the grouping set puts a NULL back into the value column. Every expected value agrees with DuckDB 1.5.2. Two unit tests cover the mark join and the anti join. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
adriangb
force-pushed
the
in-exists-subquery-projection
branch
from
September 19, 2026 17:39
b2ca096 to
7bb69a5
Compare
Contributor
Author
|
Thank you, this is a real regression and I fixed it in 7bb69a5. All three of your queries now give the base results, and the third one, which is wrong on Your diagnosis is right, and I took your -- ROLLUP adds a NULL row for `i.k`, so a miss is UNKNOWN
SELECT o.k, o.k IN (SELECT i.k FROM i WHERE i.k = o.k GROUP BY ROLLUP(i.k)) AS m FROM o;
-- an unmatched row of `a` gives a NULL `b.y`, so a miss is UNKNOWN
SELECT o.k, o.k IN (SELECT b.y FROM a LEFT JOIN (SELECT * FROM b WHERE b.y = o.k) AS b ON a.id = b.id) AS m FROM o;So the flag is cleared again when the pull up passes an outer join, a union or a grouping set. fn f_up(&mut self, plan: LogicalPlan) -> Result<Transformed<LogicalPlan>> {
if self.in_predicate_is_correlation && plan_may_add_null_rows(&plan) {
self.in_predicate_is_correlation = false;
}Tests, as asked, in I also ran a differential fuzz against DuckDB over 18 correlated One note on the second shape above. Pulling a correlated filter out from under the nullable side of an outer join is wrong on |
adriangb
requested review from
jayzhan211
and
a balanced review from Copilot
September 19, 2026 17:48
adriangb
force-pushed
the
in-exists-subquery-projection
branch
from
September 19, 2026 23:10
4765815 to
984bfbd
Compare
The rule decided null-awareness with three special cases: a flag that `PullUpCorrelatedExpr` set when it dropped a duplicated correlation and cleared again for a list of plan nodes, a switch to a weaker column test when the null-aware `LeftAnti` executor could not take the join, and an empty key list that selected that weaker test at the constant `IN` call. Replace them with one question. `IN` can only be UNKNOWN when its value or the subquery output can be NULL inside the scope of an outer row. A correlated conjunct such as `y = x` is never TRUE for a NULL, so a key it compares cannot be NULL in scope. `PullUpCorrelatedExpr` now records every correlated conjunct before it drops the duplicated one, and `JoinKeys` asks the two sides of the `IN` predicate for their nullability against those conjuncts. A correlation key that is NULL only empties the scope, so it never makes the join null-aware. The executor limits are then applied in one place. A `NOT IN` whose value can be NULL and whose join has more than one key becomes a null-aware `LeftMark` join, which takes any number of keys, plus a filter on the mark. A `NOT IN` with a residual filter becomes the mark joins that materialize its three-valued result, the same plan as a projected `IN`. Both shapes failed to plan or gave wrong results before. The constant value projection now applies to correlated subqueries too, since the mark join accepts the correlation as a second key. The guard for a grouping set above the dropped filter is gone with the flag. That query is wrong on `main` for `EXISTS` as well, because the pull up moves the filter above the aggregate; that is apache#25519. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
adriangb
force-pushed
the
in-exists-subquery-projection
branch
from
September 19, 2026 23:11
984bfbd to
fb08efa
Compare
`cargo doc` runs with `-D warnings`, and the doc comment of the public field `correlated_filters` made an intra-doc link to `remove_duplicated_filter`, which is private. The link resolves only because CI passes `--document-private-items`, so rustdoc reports `rustdoc::private_intra_doc_links` and the job fails. Name the function in backticks instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`joins.slt` pins the plan of this shape, but no test gave its result. The tables there hold no NULL in the subquery column, so `main` answers that query correctly and the plan change alone looks like a cost with no gain. Put one NULL in the subquery column and `main` drops every row: its null-aware anti join does not apply the residual filter when it looks for a NULL, so it treats a subquery that is empty for most outer rows as one that holds a NULL for all of them. DuckDB 1.5.2 gives the two rows that this test expects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The fallback for an `IN` whose join keeps a residual filter built three mark joins. The first says whether the value matched. The other two, both without a join predicate, say whether the subquery gives a NULL and whether it gives any row at all, and the caller combined them as `has_null OR (value IS NULL AND non_empty)`. One join answers both. Its filter keeps a subquery row when the subquery value is NULL, or when the outer value is NULL and the row is in scope at all. For a value that is not NULL the mark reads `has_null`. For a value that is NULL it reads `non_empty`, which is the weaker fact that case needs and which `has_null` implies. `IS NULL` is two-valued on both sides, so neither test adds an UNKNOWN of its own. `build_join` gains the extra predicate, and `mark_join_with_alias` lets the caller own the alias so that it can name the aliased subquery column. No result changes: only `EXPLAIN` expectations move, and the `CASE` loses its disjunction. A differential fuzz of 560 correlated `IN` and `NOT IN` queries against DuckDB 1.5.2 finds no mismatch, and the same fuzz finds 98 on the merge base. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… rule The merged UNKNOWN join names the value in its join filter. When that value is an outer reference of an enclosing subquery it cannot resolve there, and `build_join` reads it as a constant because `column_refs` does not report an outer reference. The query is not supported either way, but the optimizer reported a schema error instead of the plain "not implemented" that the physical planner gives. Decline the rewrite for such a value, which restores that message for a scalar subquery and for an `IN` that is nested in one. An `EXISTS` that holds the same predicate still reports a schema error; it is unsupported on `main` as well, and is not made worse by the merge beyond the text. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
This PR supersedes #21363 by @crm26. It reuses the single mark join approach from that PR, and @crm26 is a co-author of the main commit.
It also builds on #24972, which added the decorrelation of
INsubqueries in a projection.Rationale for this change
An
INorNOT INsubquery in a SELECT list gives correct results today, but the plan is quadratic. The optimizer builds three mark joins for each subquery. Two of them have no join predicate, so they run as nested loop joins over all outer rows and all inner rows.COALESCEduplicates the expression, so that shape gets six mark joins.This PR keeps one hash mark join for each subquery when the join filter is hashable. The three join fallback stays only for a non-equality correlation (Q6 below), which is not changed by this PR and is tracked separately in #25336.
INin the SELECT listCOALESCE((x IN (...))::boolean, false)INwith an equality predicateINsubqueries in separate columnsEXISTSINwith a non-equality predicate (fallback path, unchanged)PR to add these as benchmarks: #25346
Reproduction with datafusion-cli: script, plans and timings for each shape
Run the script with
datafusion-cli -f mre.sql. It creates two tables with 200000 rows each, then for each shape it printsEXPLAINand runs the query.datafusion-cliprints the elapsed time after each statement. The numbers above come from one run of this script on an Apple Silicon laptop,mainat 22651d2 and this PR built withcargo build -p datafusion-cli --profile ci. The result rows of every query are identical between the two builds.Q1: Bare `IN` in the SELECT list, plans on main and on this PR
main, query time 190.730 s
this PR, query time 0.034 s
Q2: `COALESCE((x IN (...))::boolean, false)`, plans on main and on this PR
main, query time 351.579 s
this PR, query time 0.054 s
Q3: Correlated `IN` with an equality predicate, plans on main and on this PR
main, query time 1.372 s
this PR, query time 0.091 s
Q4: Two `IN` subqueries in separate columns, plans on main and on this PR
main, query time 296.676 s
this PR, query time 0.063 s
Q5: Correlated `EXISTS`, plans on main and on this PR
main, query time 0.021 s
this PR, query time 0.021 s
Q6: Correlated `IN` with a non-equality predicate (fallback path, unchanged), plans on main and on this PR
main, query time 307.565 s
this PR, query time 261.582 s
What changes are included in this PR?
build_joinnow reports whether the mark column of theLeftMarkjoin is exact under three-valued logic. It is exact when the join filter is hashable only. The join is null aware when the keys can be NULL.in_subquery_value_mark_joinindatafusion/optimizer/src/decorrelate_predicate_subquery.rsnow builds thematchedjoin first. If that join is exact, it returns the mark column alone, orNOT markforNOT IN. The three join materialization is kept only when a residual non-equality filter remains, because the hash join cannot mark UNKNOWN for a residual predicate.The regression test for fix: scan subqueries when advancing the extracted-alias generator #24574 in
projection_pushdown.sltnow uses a correlated subquery withLIMIT 1. The subquery then still reachesExtractLeafExpressions, and the alias generator still starts at 2 inside it. The earlier version of the test let the subquery be flattened, so it no longer exercised the scan inside a subquery path.New sqllogictest cases in
subquery_projection.slt:EXPLAINguard that shows one hash mark join for each subquery,EXPLAINguard that shows the three join fallback when a residual filter remains,INandNOT INwith a NULL in the subquery, a NULL outer value,NOTinsideCASE, a projection over an aggregate, theCOALESCEand cast shape, and results for the residual filter shape.All of these were checked against DuckDB and PostgreSQL.
join_keys_may_be_nullnow decides null-awareness from the equijoin key expressions, not from the columns they reference. A key such asNULLIF(id, 1)orTRY_CAST(s AS INT)can beNULLover aNOT NULLcolumn. Before this change such a key gave a plain mark join, so the projectedINreturnedfalseinstead ofNULL. The same helper feeds theLeftAntipath, soWHERE NULLIF(id, 1) NOT IN (SELECT ...)onmainkept a row that it must drop. Both paths now plan a null-aware join for these keys.The expression test applies only when the split gives keys and no residual filter. A residual keeps the older column test, as more than one key already does. The reason is the
LeftAntiexecutor: it does not apply the residual when it decides whether a NULL makes the result UNKNOWN (Wrong results: correlated NOT IN with a non-equality correlation returns no rows (null-aware LeftAnti join ignores the residual filter) #25336), so it would drop a build row whose correlated subquery result is empty, andNULL NOT IN (<empty set>)is TRUE.subquery_projection.sltpins both directions of that shape. Every result of this branch is thus the same asmain's or better.DuckDB comparison
For an absolute reference, the same seven queries in
datafusion-cliand in DuckDB 1.5.2, on the same two tables, release build, Apple M4 Pro.mainis the merge-base64871d9and "this PR" is3af87370c1. The tables are the ones the suite builds, at its default of 30,000 rows each. Each number is the median of 7 runs. One round runs one session per engine, and the rounds interleave the three engines.mainINCOALESCEoverININ, equalityINcolumnsNOT INEXISTSIN, non-equalityAll three engines give the same result for all seven queries, and the counts are the ones in the suite's checked-in result files.
For the four quadratic shapes, q01, q02, q04 and q05,
mainis 270x to 680x slower than DuckDB. This PR puts them at DuckDB's cost. These times are at the 1 ms resolution of both command line tools, so the exact factor is approximate; the size of the difference is not.q07 keeps the three-join plan, which this PR does not change. DataFusion is 4.9x faster than DuckDB there, before and after.
A second run at 100,000 rows per table gives the same picture:
maintakes 2741 ms to 6355 ms for q01, q02, q04 and q05, this PR takes 2 ms to 4 ms and DuckDB 3 ms to 4 ms, and q07 takes 971 ms against DuckDB's 1534 ms. All three engines again give the same counts.How to run it
The queries are the suite's own
benchmarks/sql_benchmarks/projection_subquery/queries/q0*.sql, with an alias added to the derived table. The load SQL needs one change for DuckDB, becausegenerate_seriesgives a column of that name there and a column namedvaluein DataFusion:datafusion-clireports the time of each statement asElapsed, andduckdbreports it asRun Time (s): realafter.timer on.What is the testing strategy for this PR?
datafusion/optimizer/src/decorrelate_predicate_subquery.rs. The updated snapshots now show a single mark join. There are new tests for the residual filter fallback and forNOT IN.subquery_projection.sltlisted above, checked against DuckDB and PostgreSQL.subquery_projection.sltcases pass without any change.subquery_projection.sltsection cover nullable key expressions over non-nullable columns: aNULLIFleft key, aTRY_CASTleft key, aNULLIFright key,NOT IN, and theWHEREform. The expected values agree with DuckDB and PostgreSQL.Are there any user-facing changes?
There are no changes to query results and no changes to any public API. Plans for projected
INandNOT INsubqueries are much faster. No documentation change is needed.🤖 Generated with Claude Code