comphead opened a new pull request, #26544:
URL: https://github.com/apache/datafusion/pull/26544
## Which issue does this PR close?
- Closes #26508.
## Rationale for this change
An aggregate without `GROUP BY` returns one row even when its input is
empty. When such an aggregate is in a correlated subquery and another aggregate
sits above it, DataFusion returns wrong results for outer rows that match no
inner rows:
```sql
SELECT x.a,
(SELECT count(*) FROM (SELECT max(y.b) FROM (VALUES (1, 10)) AS y(a,
b) WHERE y.a = x.a)) AS v1,
(SELECT max(n) FROM (SELECT count(*) AS n FROM (VALUES (1, 10)) AS
y(a, b) WHERE y.a = x.a)) AS v2
FROM (VALUES (1), (2)) AS x(a)
```
For `a = 2` this returns `v1 = 0, v2 = NULL` instead of `v1 = 1, v2 = 0`,
which DuckDB, SQLite and PostgreSQL return. The same shape gives wrong results
under `IN` and `LEFT JOIN LATERAL` too.
When the correlated filter is pulled up, the inner scalar aggregate becomes
a grouped aggregate, which returns no row for the outer rows it does not match.
The aggregate above then runs over an empty input. The count bug handling only
fixes the aggregate the join reads, which here is the outer one, so the inner
row is never restored. The existing `TODO` next to `pulled_up_scalar_agg`
described this case.
## What changes are included in this PR?
`PullUpCorrelatedExpr` stops the pull up when it reaches an `Aggregate`
after a scalar aggregate has already been pulled up. The subquery stays
correlated, the same way a window over an aggregate with count bug handling
does today. Such a query now fails with `Physical plan does not support logical
expression ScalarSubquery` instead of returning wrong rows.
A projection over the scalar aggregate (`SELECT n + 1 FROM (SELECT count(*)
AS n ...)`) and an aggregate over a grouped aggregate are still pulled up.
## What is the testing strategy for this PR?
New `sqllogictest` cases at the end of `subquery.slt` cover the two queries
from the issue, the `IN` and `LATERAL` forms, and the two shapes that must
still be pulled up. All `sqllogictest` files, the `datafusion-optimizer` unit
tests and the `core_integration` `sql::` tests (which include the TPC-DS
planning tests) pass locally.
## Are there any user-facing changes?
Queries with this shape now return a "not implemented" error instead of
wrong results. No API changes.
--
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]
