MIME-Version: 1.0
References: <19533-413a1014e5d0e766@postgresql.org>
 <EC273387-5C73-4C9C-BF30-8BA260A90FFB@outlook.com>
 <CAMbWs4-6umOecRifmC88RW0SHWDAvb4c3-VD8XQ3NJufsUyUOQ@mail.gmail.com>
In-Reply-To: 
 <CAMbWs4-6umOecRifmC88RW0SHWDAvb4c3-VD8XQ3NJufsUyUOQ@mail.gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
Date: Tue, 30 Jun 2026 01:44:20 +1200
Message-ID: 
 <CAApHDvqHXZpF9pPStLqPp5O=G2gVpPD959bMTRRkLjdy4Tvx7g@mail.gmail.com>
Subject: Re: BUG #19533: Wrong results from WindowAgg run-condition pushdown
 on count() with EXCLUDE CURRENT ROW
To: Richard Guo <guofenglinux@gmail.com>
Cc: Chengpeng Yan <chengpeng_yan@outlook.com>,
 "imchifan@163.com" <imchifan@163.com>,
	"pgsql-bugs@lists.postgresql.org" <pgsql-bugs@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAApHDvqHXZpF9pPStLqPp5O%3DG2gVpPD959bMTRRkLjdy4Tvx7g%40mail.gmail.com>
Precedence: bulk

On Mon, 29 Jun 2026 at 12:39, Richard Guo <guofenglinux@gmail.com> wrote:
> I wonder if we should still support count(*) with EXCLUDE CURRENT ROW,
> as that always removes exactly one row, so it is still monotonic.
>
> +   if ((frameOptions & FRAMEOPTION_EXCLUSION) &&
> +       ((frameOptions & FRAMEOPTION_EXCLUSION) !=
> FRAMEOPTION_EXCLUDE_CURRENT_ROW ||
> +        req->window_func->winfnoid != F_COUNT_))
> +       monotonic = MONOTONICFUNC_NONE;

Yeah, I noticed that one too. I think there are a few other cases we
could also allow.  I'm still looking, but I think COUNT(*) could still
be monotonically increasing when frame_end is CURRENT ROW for any of
EXCLUDE (CURRENT ROW|GROUP|TIES). In all those cases, since we've not
looked at any rows beyond the current row, we're not susceptible to
the exclude option excluding rows from a higher-order peer group,
which may have more rows to count.

I think there might be another with COUNT(ANY): "ORDER BY k RANGE
BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW EXCLUDE GROUP". Here we're
excluding the entire group, so the count can't go backwards within
that group as we don't count anything due to EXCLUDE GROUP, and since
frame_end is CURRENT ROW, we don't have to concern ourselves with
counting some later group which has more peer rows making the count go
backwards (when that larger number of rows is excluded later when
processing that peer group).

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.

David