Re: BUG #19533: Wrong results from WindowAgg run-condition pushdown on count() with EXCLUDE CURRENT ROW

On Tue, 30 Jun 2026 at 01:44, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> I think there are also a few equivalences, such as:
>
> "order by k rows between unbounded preceding and current row exclude
> current row"
>
> I believe is logically the same as:
>
> "order by k rows between unbounded preceding and 1 preceding"
>
> Then, if frame_end was 1 PRECEDING with "GROUPS BETWEEN ...", EXCLUDE
> would never see the current row or peer group as the frame ends before
> we get to the current group or row.
>
> Anyway, I'm still trying to decide how to balance this. On one hand,
> there's an incentive not to disable this optimisation for cases that
> are perfectly safe, and on the other hand, there are lots of variables
> here, and "perfectly safe" is a little harder to fathom than I'd like
> for a bug fix. I'm also not keen on adding very complex and
> hard-to-follow logic to the support function. If we can allow a few
> more cases but do so as a result of boiling down the logic into
> something simpler, then I'd likely be more in favour of covering some
> extra cases.
>
> I'll continue to look at this.

I've been playing around with the attached. It doesn't get all the
cases that are ok. For example, in theory, I believe 0 FOLLOWING is
the same as CURRENT ROW for the frame_end. I've not made that work
exactly the same as I don't have the same wfunc->winfnoid ==
F_COUNT_ANY check that I added to CURRENT ROW for 0 FOLLOWING. I was
trying to strike some sort of balance between not regressing things
unnecessarily and having to add too much code for cases that are
unlikely ever to be used. As far as I'm aware, for GROUPS and ROWS
mode, 0 FOLLOWING is the same as CURRENT ROW, so I suspect most people
would just use CURRENT ROW.

It's a little hard for me to know what to do here, as I really don't
know how commonly people use the EXCLUDE clause, and more so, how
often the Run Condition optimisation will apply when those are used.
It would be good to have an idea if the bug discovery was from a
real-world case, or if it was discovered from tooling, or from looking
at the code.

Does anyone think we should cover more or fewer cases? Or see any
problems with this patch?

David