jayzhan211 commented on code in PR #26544:
URL: https://github.com/apache/datafusion/pull/26544#discussion_r4336385250
##########
datafusion/optimizer/src/decorrelate.rs:
##########
@@ -505,6 +505,12 @@ impl TreeNodeRewriter for PullUpCorrelatedExpr {
if !is_distinct {
self.can_pull_up = self.can_pull_up &&
self.can_pull_over_aggregation;
}
+ // A pulled up scalar aggregate below gives no row for
unmatched
+ // outer rows, so this aggregate would see an empty input.
+ // See https://github.com/apache/datafusion/issues/26508
+ if self.pulled_up_scalar_agg {
Review Comment:
This also blocks some shapes that were correct before and now fail to plan.
For example, `max` over `max`, `sum` over `sum` and `count(m)` over `max` all
returned the right rows on main, and now hit `Physical plan does not support
logical expression ScalarSubquery`:
```sql
SELECT o.k, (SELECT max(m) FROM (SELECT max(i.v) AS m FROM nested_agg_inner
AS i WHERE i.id = o.k))
FROM nested_agg_outer AS o ORDER BY o.k;
-- main: (1, 10), (2, NULL) this PR: error
```
Blocking makes sense to me, since a precise rule would have to evaluate the
outer aggregate on the inner aggregate's empty-input row. Could you mention
this in the PR description and add a test that pins the new error, so it's
clear this is an intended trade-off?
##########
datafusion/sqllogictest/test_files/subquery.slt:
##########
@@ -3627,3 +3627,45 @@ DROP TABLE unnest_outer;
statement ok
DROP TABLE unnest_inner;
+
+# An aggregate over a correlated scalar aggregate stays correlated.
+# https://github.com/apache/datafusion/issues/26508
+statement ok
+CREATE TABLE nested_agg_outer(k INT) AS VALUES (1), (2);
+
+statement ok
+CREATE TABLE nested_agg_inner(id INT, v INT) AS VALUES (1, 10);
+
+# Expected `(1, 1), (2, 1)` and `(1, 1), (2, 0)`.
+statement error DataFusion error: This feature is not implemented: Physical
plan does not support logical expression ScalarSubquery
+SELECT o.k, (SELECT count(*) FROM (SELECT max(i.v) FROM nested_agg_inner AS i
WHERE i.id = o.k)) FROM nested_agg_outer AS o ORDER BY o.k;
+
+statement error DataFusion error: This feature is not implemented: Physical
plan does not support logical expression ScalarSubquery
+SELECT o.k, (SELECT max(n) FROM (SELECT count(*) AS n FROM nested_agg_inner AS
i WHERE i.id = o.k)) FROM nested_agg_outer AS o ORDER BY o.k;
+
+# IN and LATERAL. Expected `2` and `(1, 1), (2, 0)`.
Review Comment:
nit: Two more shapes that returned wrong rows on main are now blocked too:
`EXISTS`, and a projection between the two aggregates. It would be good to
cover them here:
```diff
+# EXISTS. Expected `1, 2`.
+statement error DataFusion error: This feature is not implemented: Physical
plan does not support logical expression Exists
+SELECT o.k FROM nested_agg_outer AS o WHERE EXISTS (SELECT max(n) FROM
(SELECT count(*) AS n FROM nested_agg_inner AS i WHERE i.id = o.k)) ORDER BY
o.k;
+
+# A projection between the aggregates. Expected `(1, 2), (2, 1)`.
+statement error DataFusion error: This feature is not implemented: Physical
plan does not support logical expression ScalarSubquery
+SELECT o.k, (SELECT max(n2) FROM (SELECT n + 1 AS n2 FROM (SELECT count(*)
AS n FROM nested_agg_inner AS i WHERE i.id = o.k))) FROM nested_agg_outer AS o
ORDER BY o.k;
```
--
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]
