Repository navigation
fix: keep a LIMIT inside EXISTS that an operator above it can empty - #26640
Open
raghavyadav01 wants to merge 1 commit into
Open
raghavyadav01 wants to merge 1 commit into
raghavyadav01 wants to merge 1 commit into
Conversation
`PullUpCorrelatedExpr::f_down` reasoned that "for EXISTS only emptiness
matters" and dropped any `Limit` with `skip=0` and a non-zero fetch from
an `EXISTS` subquery.
That holds for the emptiness of the `Limit`'s own output. What `EXISTS`
reads is whether the whole subquery is empty, so the argument breaks as
soon as a node above the `Limit` can be empty for a reason of its own:
SELECT t1_id FROM t1 WHERE EXISTS (
SELECT 1 FROM (SELECT t2_id FROM t2 WHERE t2_int = t1_int LIMIT 1) s
HAVING count(*) > 1
);
`count(*)` over the single row `LIMIT 1` keeps can never exceed 1,
whichever row that is, so the `HAVING` holds for nothing and the query
returns no rows. Dropping the `Limit` lets the aggregate count the whole
scan, and on the `subquery.slt` tables this returned `33`. A `Filter`
above the `Limit` goes wrong the same way.
Track, per node on the path from the subquery's root, whether the
emptiness of that node's output follows from the emptiness of its input
alone, and only drop the `Limit` when that holds of every node above it.
The condition is emptiness, not row count. A `Projection`, a
`SubqueryAlias`, a `Sort`, a `Window` and a `Repartition` each output one
row per input row. A `DISTINCT` and a grouped `Aggregate` output rows
exactly when their input has rows, and an `Aggregate` without a grouping
always outputs its one row, so no `Aggregate` can be emptied by the rows
a `Limit` held back - `EXISTS (SELECT count(*) FROM (.. LIMIT 1) s)`,
`EXISTS (SELECT DISTINCT .. FROM (.. LIMIT 1) s)` and a `GROUP BY` above
the limit all keep decorrelating, as does the common
`EXISTS (SELECT 1 FROM t WHERE t.a = o.a LIMIT 1)`. A `Filter` is what
this leaves out, together with the nodes that can empty a non-empty
input: `Join`, `Union`, `Unnest`.
Where the `Limit` now has to stay, the subquery does not decorrelate and
the query fails to plan. That is the same trade `subquery.slt` already
records for `EXISTS (SELECT * FROM t2 WHERE t2_id = t1_id offset 1)`,
which is asserted to error "rather than returning wrong rows", and for
the equivalent correlated `IN` subquery with a `LIMIT`.
`LIMIT 0` keeps its existing rewrite to an empty relation, which is
sound whatever sits above it: both sides are always empty.
Tests go in `subquery.slt` next to the existing `offset` cases: the two
shapes that were wrong, and the four that must keep working. The three
new assertions all fail on `main`. The rule's own unit tests cover the
decorrelated plan shape. The full sqllogictest run fails in the same 236
places before and after.
Fixes apache#26629
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Raghvendra Yadav <raghavmnnit@gmail.com>
raghavyadav01
force-pushed
the
fix/exists-limit-below-row-counting-node
branch
from
October 24, 2026 20:14
1dd5a07 to
1cf3189
Compare
Contributor
Author
|
Rebased past #26551 — no conflict; that change is in the @jayzhan211 would you have time to look? You merged #26551 and #25781 in this area. The change is narrow: |
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?
Rationale for this change
PullUpCorrelatedExpr::f_downreasoned that "for EXISTS only emptiness matters" and dropped anyLimitwithskip=0and a non-zero fetch out of anEXISTSsubquery.That holds for the emptiness of the
Limit's own output. WhatEXISTSreads is whether thewhole subquery is empty, so the argument breaks as soon as a node above the
Limitcan beempty for a reason of its own. On the
subquery.slttables:count(*)over the single rowLIMIT 1keeps can never exceed 1, whichever row that is, so theHAVINGholds for nothing and the query returns no rows. Dropping theLimitlets the aggregatecount the whole scan, and on
mainthis returns33. AFilterabove theLimitgoes wrong thesame way.
What changes are included in this PR?
PullUpCorrelatedExprnow tracks, per node on the path from the subquery's root, whether theemptiness of that node's output follows from the emptiness of its input alone, and only drops the
Limitwhen that holds of every node above it. The entry is pushed and popped in lockstep withthe existing
column_refs_above.The condition is emptiness, not row count, which matters for how little this rejects. A
Projection, aSubqueryAlias, aSort, aWindowand aRepartitioneach output one row perinput row. A
DISTINCTand a groupedAggregateoutput rows exactly when their input has rows,and an
Aggregatewithout a grouping always outputs its one row — so noAggregatecan beemptied by the rows a
Limitheld back. These all keep decorrelating:Filteris what the predicate leaves out, together with the nodes that can empty a non-emptyinput:
Join,Union,Unnest. Anything not listed is treated as unsafe, so a newLogicalPlanvariant does not silently become droppable.
LIMIT 0keeps its existing rewrite to an empty relation, which is sound whatever sits above it,since both sides are always empty.
The trade-off worth reviewing
Where the
Limitnow has to stay, the subquery does not decorrelate and the query fails to planwith
Physical plan does not support logical expression Exists(..).That is the trade
subquery.sltalready records a few lines above these new tests, forEXISTS (SELECT * FROM t2 WHERE t2_id = t1_id offset 1), asserted to error "rather thanreturning wrong rows" — and
maindoes the same for the equivalent correlatedINsubquery witha
LIMIT. So this puts the two broken shapes into an existing, already-asserted bucket ratherthan inventing a new behaviour. Happy to go further if you would rather these decorrelate properly
via the
row_number()route the issue mentions; that looked like a much larger change and I didnot want to bundle it.
What is the testing strategy for this PR?
datafusion/sqllogictest/test_files/subquery.slt, in the existingEXISTS-with-Limit/offsetblock: the two shapes that returned wrong answers, and the four above that must keep working.
The three new assertions all fail on
main— theexplainshowsmaindropping theLimitinto a
LeftSemijoin, and both error assertions fail becausemainreturns rows.The rule's own unit tests in
decorrelate_predicate_subquery.rspin the decorrelated plan shapefor a
Limitunder aProjectionand under anAggregate, and that theHAVINGandFiltershapes are left untransformed.
cargo test -p datafusion-optimizer: 951 pass.sqllogictests: fails in the same 236 places before and after this change, identical sets.cargo clippy --all-targets --workspace --features avro,integration-tests,extended_tests -- -D warnings: clean.cargo fmt --all --check,typos, andRUSTDOCFLAGS="-D warnings" cargo doc --document-private-items: clean.🤖 Generated with Claude Code