comphead commented on code in PR #26013:
URL: https://github.com/apache/datafusion/pull/26013#discussion_r4338752701
##########
datafusion/optimizer/src/decorrelate.rs:
##########
@@ -664,6 +674,16 @@ 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. A window over an
+ // aggregate with count bug handling stops the pull up in
+ // `pass_through_pulled_up_cols`.
+ if self.can_extend_partition_by {
Review Comment:
Thanks @kumarUjjawal. Reproduced both on `b6debc1c6`, and found that
`EXISTS`, `NOT EXISTS` and `IN` had the same gap, since they never record count
bug state. For example `EXISTS (SELECT count(*) OVER () FROM (SELECT count(*)
... WHERE i.y = o.k))` returned `1, 2` instead of `1, 2, 5`.
Rather than preserving the state through each node, the window now stays
correlated whenever the pull up converted an aggregate without `GROUP BY`
(`pulled_up_scalar_agg`), which is set for every subquery kind. A window over a
grouped aggregate is still partitioned.
New `subquery.slt` cases: `DISTINCT`, a `Filter` that cannot be evaluated on
an empty input (`random() >= 0`), `EXISTS`, `NOT EXISTS`, `IN` and `LATERAL`
over a scalar aggregate, which stay correlated as on the merge base, and a
grouped aggregate below the window, which returns results. The first five fail
without the fix.
--
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]
