fornwall opened a new pull request, #25284:
URL: https://github.com/apache/datafusion/pull/25284
## Which issue does this PR close?
Closes #25283.
## Rationale for this change
A correlated `EXISTS` subquery with an `OFFSET` returns wrong results: the
`OFFSET` is silently dropped when the subquery is rewritten into a semi join.
```sql
CREATE TABLE t1(k INT) AS VALUES (1), (2), (3);
CREATE TABLE t2(v INT) AS VALUES (1), (1), (3);
SELECT k FROM t1 WHERE EXISTS (SELECT * FROM t2 WHERE t2.v = t1.k OFFSET 1);
```
Only `k = 1` has two matching rows in `t2`, so only for `k = 1` is a row
left after skipping one. The expected result is a single row with `1`, which is
also what SQLite returns. DataFusion returns `1` and `3`. The `NOT EXISTS` form
is wrong in the same way: it returns only `2`, but `2` and `3` are expected.
`EXPLAIN` shows why. The subquery becomes a plain `LeftSemi` join with no
`Limit` node at all:
```
LeftSemi Join: t1.k = __correlated_sq_1.v
TableScan: t1 projection=[k]
SubqueryAlias: __correlated_sq_1
TableScan: t2 projection=[v]
```
To rewrite the subquery into a semi join, `PullUpCorrelatedExpr` removes the
`LIMIT` that sits above the correlated filter, on the grounds that `EXISTS`
only asks whether any row exists and a `LIMIT n` with `n > 0` cannot change
that. The same plan node carries the `OFFSET`, which was dropped along with it,
although an offset can turn a non-empty subquery into an empty one.
## What changes are included in this PR?
`PullUpCorrelatedExpr` now only removes the limit of a correlated `EXISTS`
subquery when it cannot change whether the subquery is empty: a zero offset
with a literal fetch. A zero fetch still becomes an empty relation. 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 correlated in the plan and is reported as
unsupported rather than answered wrongly.
## What is the testing strategy for this PR?
New sqllogictest cases in `subquery.slt`:
- `exists_subquery_with_offset0` checks that `LIMIT 1 OFFSET 0` is still
decorrelated.
- `exists_subquery_with_offset` and `not_exists_subquery_with_offset` check
that `OFFSET 1` keeps the subquery correlated, and that running the query fails
with a not-implemented error. The explain cases fail without the fix.
## Are there any user-facing changes?
A correlated `EXISTS` or `NOT EXISTS` subquery with a positive `OFFSET` now
fails with a not-implemented error instead of returning wrong results. Queries
with a zero offset are unaffected.
A correlated `EXISTS` subquery whose `LIMIT` is not a literal (which could
evaluate to zero) previously had that limit dropped as well; it now fails with
the same not-implemented error.
Disclaimer: Created with fable 5.1 in claude code. I have reviewed the code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
