Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25284 +/- ##
==========================================
- Coverage 81.92% 81.92% -0.01%
==========================================
Files 1135 1135
Lines 427573 427583 +10
Branches 427573 427583 +10
==========================================
- Hits 350279 350278 -1
- Misses 56367 56374 +7
- Partials 20927 20931 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
fornwall
force-pushed
the
offset-of-correlated-exists
branch
8 times, most recently
from
September 14, 2026 02:20
9b21f10 to
ff7c7c2
Compare
A correlated subquery refers to columns of the query it is nested in, so in principle it has to be evaluated once per outer row: ```sql SELECT k FROM t1 WHERE EXISTS (SELECT * FROM t2 WHERE t2.v = t1.k) ``` Re-running the subquery for every outer row would be slow, and DataFusion has no operator that does so. Instead the optimizer rewrites it into a join, which is how analytical engines execute correlated subqueries efficiently: the condition that mentions the outer column (`t2.v = t1.k`) is pulled up out of the subquery and becomes the join condition of a semi join between `t1` and `t2`. Any operator that sits between that condition and the top of the subquery is in the way of the pull up, and a `LIMIT` is one such operator. For an `EXISTS` subquery a limit was simply deleted to clear the way, because `EXISTS` only asks whether the subquery returns any row, and a `LIMIT n` with `n > 0` cannot change that (a `LIMIT 0` was turned into an empty relation). This overlooked that the same plan node also carries the `OFFSET`, and an offset does change the answer: it skips rows, so a subquery that would return one row returns none after `OFFSET 1`. The offset was dropped along with the fetch and the query answered wrongly. With two rows of `v = 1` and one row of `v = 3` in `t2` ```sql SELECT k FROM t1 WHERE EXISTS (SELECT * FROM t2 WHERE t2.v = t1.k OFFSET 1) ``` returned both `1` and `3`, although only `k = 1` has a second row to skip past. Only remove the limit when it cannot change whether the subquery is empty, which is a zero offset with a literal fetch. A zero fetch still becomes an empty relation. Anything else, a positive offset or an offset or fetch that is not a literal, marks the subquery as one that cannot be pulled up, so it stays a correlated subquery in the plan and is reported as unsupported rather than answered wrongly. The decision to remove a limit was also made from the wrong information. It keyed on whether any correlated condition had been collected so far in the subquery, not on whether one sat below this particular limit. In a join inside the `EXISTS` the correlated side is visited first, so a `LIMIT` on an unrelated, uncorrelated sibling branch was deleted too and changed the result. Decide per limit instead, in `f_down`, from the outer references of that limit's own subtree, which is how `IN` subqueries were already handled. It has to happen in `f_down`: by `f_up` the filters below have been rewritten and their outer references are gone. The limit is rewritten right there in `f_down`, and the unsupported shapes now bail in `f_down` like the other unsupported shapes in this rewriter. Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
fornwall
force-pushed
the
offset-of-correlated-exists
branch
from
September 15, 2026 14:46
ff7c7c2 to
c666d06
Compare
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?
Closes #25283.
Rationale for this change
A correlated
EXISTSsubquery with anOFFSETreturns wrong results, because theOFFSETis dropped when the subquery is rewritten into a semi join:Only
k = 1has two matching rows, so the expected result is a single row with1(as SQLite returns). DataFusion returns1and3.NOT EXISTSis wrong the same way.PullUpCorrelatedExprremoves theLIMITabove the correlated filter of anEXISTSsubquery, sinceLIMIT nwithn > 0cannot change whether any row exists. But the same node carries theOFFSET, which can turn a non-empty subquery into an empty one.The decision to remove a
Limitwas also keyed on whether any correlated predicate had been collected so far, not on whether one sat below thisLimit. In a join inside theEXISTSthe correlated side is visited first, so aLIMITon an uncorrelated sibling branch was removed too, changing the result:With the
subquery.sltfixtures this returned11, 22, 44instead of11, 44.What changes are included in this PR?
PullUpCorrelatedExprnow decides perLimit, inf_down, whether it sits above correlated expressions, using the outer references of its own subtree (as it already did forINsubqueries), and rewrites theLimitright there.For a correlated
EXISTS, theLimitis only removed when it cannot change whether the subquery is empty: a zero offset with a literal fetch. A zero fetch still becomes an empty relation. Anything else (a positive or non-literal offset, or a non-literal fetch) marks the subquery as not pull-up-able, so it stays correlated and fails as unsupported instead of returning wrong rows.A
Limitwith no correlated expressions below it is left alone.What is the testing strategy for this PR?
New sqllogictest cases in
subquery.slt:LIMIT 1 OFFSET 0is still decorrelated.OFFSET 1keeps the subquery correlated, and running the query fails with a not-implemented error. The explain case fails without the fix. The same forNOT EXISTS, and forEXISTSin a disjunction.LIMIT 0 OFFSET 1still becomes an empty relation.LIMITkeeps the subquery correlated.LIMITorOFFSETon an uncorrelated branch of a join inside theEXISTSis kept and the query returns the correct rows. Both fail without the per-Limitdecision.Are there any user-facing changes?
EXISTS/NOT EXISTSsubquery with a positiveOFFSET, or with a non-literalLIMIT, now fails with a not-implemented error instead of returning wrong results (previously theLIMIT/OFFSETwas silently dropped). I can file an issue and take a stab at supporting these in a follow-up once this is merged.LIMITorOFFSETon an uncorrelated branch inside a correlatedEXISTSsubquery is no longer dropped, so such queries now return correct results.Disclaimer: Created with fable 5.1 in claude code. I have reviewed the code.