kumarUjjawal commented on code in PR #26013:
URL: https://github.com/apache/datafusion/pull/26013#discussion_r4306661595
##########
datafusion/optimizer/src/decorrelate.rs:
##########
@@ -577,6 +581,17 @@ impl TreeNodeRewriter for PullUpCorrelatedExpr {
.iter()
.all(|expr| partitions_by_all(expr,
&local_correlated_cols))
{
+ // `inner.y = outer.k` keeps the rows of one `y` value for
+ // each outer row, so partitioning by `y` as well gives
each
+ // window function the same rows.
+ if self.can_extend_partition_by
+ &&
!self.collected_count_expr_map.contains_key(&*window.input)
Review Comment:
There is still a gap when a projection discards the aggregate column.
`SELECT 1 AS x` drops the `collected_count_expr_map` entry, so the window guard
misses it.
I reproduced this on `0092bee2a`:
```sql
CREATE TABLE o(k INT) AS VALUES (1), (2), (5);
CREATE TABLE i(y INT) AS VALUES (1), (2);
SELECT o.k,
(SELECT count(*) OVER ()
FROM (
SELECT 1 AS x
FROM (SELECT count(*) AS c FROM i WHERE i.y = o.k) a
) b) AS n
FROM o ORDER BY o.k;
```
Expected: `(1,1), (2,1), (5,1)`. This PR returns `(5,NULL)` for the last
row. The merge base rejects the query as unsupported.
The inner `count(*)` returns one row even without a match. We need to
preserve that guard through the projection and add this regression test.
--
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]
