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

Hi,

On Jun 24, 2026, at 22:41, PG Bug reporting form <noreply@postgresql.org> wrote:

The following bug has been logged on the website:

Bug reference: 19533
Logged by: Qifan Liu
Email address: imchifan@163.com
PostgreSQL version: 18.4
Operating system: Ubuntu 20.04 x86-64, docker image postgres:18.4
Description:

## PoC

```sql
DROP TABLE IF EXISTS t;

CREATE TABLE t (
id int PRIMARY KEY,
k int NOT NULL,
v int
);

INSERT INTO t(id, k, v) VALUES
(1, 1, 1),
(2, 1, NULL),
(3, 1, 1),
(4, 2, 1);

WITH
source_rows AS (
SELECT id
FROM (
SELECT id,
count(v) OVER (
ORDER BY k
RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW
EXCLUDE CURRENT ROW
) AS c
FROM t
) s
WHERE c <= 1
),
reference_rows AS (
SELECT t1.id
FROM t AS t1
WHERE (
SELECT count(t2.v)
FROM t AS t2
WHERE t2.k <= t1.k
AND t2.id <> t1.id
) <= 1
),
metamorphic_rows AS (
SELECT id
FROM (
SELECT id,
count(v) OVER (
ORDER BY k
RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW
EXCLUDE CURRENT ROW
) AS c
FROM t
) s
WHERE c + 0 <= 1
)
SELECT 'source' AS label, coalesce(array_agg(id ORDER BY id), '{}'::int[])
AS ids
FROM source_rows
UNION ALL
SELECT 'reference', coalesce(array_agg(id ORDER BY id), '{}'::int[])
FROM reference_rows
UNION ALL
SELECT 'metamorphic_no_pushdown', coalesce(array_agg(id ORDER BY id),
'{}'::int[])
FROM metamorphic_rows;

EXPLAIN (COSTS OFF)
SELECT id
FROM (
SELECT id,
count(v) OVER (
ORDER BY k
RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW
EXCLUDE CURRENT ROW
) AS c
FROM t
) s
WHERE c <= 1;
```

## Expected Behavior

The source query should return the same rows as the reference query and the
semantically equivalent metamorphic_no_pushdown query.

## Actual Behavior

The optimized query returns only `{1}`, while the reference and no-pushdown
forms return `{1,3}`.

Thanks for the report. I reproduced the wrong result.

The planner can treat count() as a monotonic window function and turn a
qual such as c <= 1 into a WindowAgg run condition. That is not safe
when the frame has an EXCLUDE option, because exclusion is applied after
the frame bounds are found and can depend on the current row or peer
group. In the reported count(v) case, NULL handling can produce values
such as 1, 2, 1, so the run condition can drop a later valid row.

This is not only about NULLs. count(*) has no NULL issue, and some
excluded-frame cases remain monotone, but not all of them. For example,
GROUPS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING EXCLUDE GROUP can
make count(*) decrease when the current peer group changes.

The attached patch takes the conservative route: int8inc_support() no
longer reports count() as monotonic when the window frame has
FRAMEOPTION_EXCLUSION. The added regression tests cover the reported
case, related EXCLUDE variants, and a count(*) case. They also check
that the excluded-frame query keeps a normal Filter instead of a
WindowAgg Run Condition.

I ran make check, which passed.

Thoughts welcome on whether this is the right boundary, or whether some
narrower excluded-frame cases should still prove monotonicity
separately.

--
Best regards,
Chengpeng Yan

On Sun, Jun 28, 2026 at 1:53 AM Chengpeng Yan <chengpeng_yan@outlook.com> wrote:

The attached patch takes the conservative route: int8inc_support() no
longer reports count() as monotonic when the window frame has
FRAMEOPTION_EXCLUSION. The added regression tests cover the reported
case, related EXCLUDE variants, and a count(*) case. They also check
that the excluded-frame query keeps a normal Filter instead of a
WindowAgg Run Condition.

I think the fix is on the right track. One nit is that it might be
better to use if/else if structure here. The other nit is that the
test case is over-heavy; it should be kept as minimal as possible.

Thoughts welcome on whether this is the right boundary, or whether some
narrower excluded-frame cases should still prove monotonicity
separately.

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;

- Richard

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

On Tue, 30 Jun 2026 at 01:44, David Rowley <dgrowleyml@gmail.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

On Fri, Jul 3, 2026 at 10:38 AM David Rowley <dgrowleyml@gmail.com> wrote:

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.

If it was from tooling, it must have stopped with the first case it
found, as is often the case. I just tried myself and found three more
apparent bugs with EXCLUDE both with and without the v1 patch. Do you
want to see the reproducers, or rethink the risk/reward ratio of
what's handled? I'm also not sure how common that clause is.

Separately, I also I believe I found a counterexample to the part
starting with /* No ORDER BY clause then all rows are peers */ :

CREATE TABLE t7 (x int);
INSERT INTO t7 VALUES (7), (8), (9);

=# SELECT * FROM
(SELECT x,
count(*) OVER (ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) c
FROM t7) s
WHERE c >= 2;
x | c
---+---
(0 rows)

=# SELECT * FROM
(SELECT x,
count(*) OVER (ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) c
FROM t7) s
WHERE c + 0 >= 2; -- baseline
x | c
---+---
2 | 2
2 | 3
(2 rows)

--
John Naylor
Amazon Web Services

On Fri, Jul 3, 2026 at 4:09 PM John Naylor <johncnaylorls@gmail.com> wrote:

=# SELECT * FROM
(SELECT x,
count(*) OVER (ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) c
FROM t7) s
WHERE c + 0 >= 2; -- baseline
x | c
---+---
2 | 2
2 | 3
(2 rows)

Sorry, this last result should be

x | c
---+---
8 | 2
9 | 3
(2 rows)

--
John Naylor
Amazon Web Services

On Fri, 3 Jul 2026 at 21:10, John Naylor <johncnaylorls@gmail.com> wrote:

If it was from tooling, it must have stopped with the first case it
found, as is often the case. I just tried myself and found three more
apparent bugs with EXCLUDE both with and without the v1 patch. Do you
want to see the reproducers, or rethink the risk/reward ratio of
what's handled? I'm also not sure how common that clause is.

Please share what you discovered. It would be good to understand if
it's just a simple missing check that can be added to handle all
three, or if there's just more complexity in this than making this
work correctly is worth.

Separately, I also I believe I found a counterexample to the part
starting with /* No ORDER BY clause then all rows are peers */ :

Thanks for finding that. Looks like I failed to consider ROWs mode for
that, where the function is only monotonically increasing. The
attached includes a fix for that.

David

On Sat, Jul 4, 2026 at 6:57 AM David Rowley <dgrowleyml@gmail.com> wrote:

On Fri, 3 Jul 2026 at 21:10, John Naylor <johncnaylorls@gmail.com> wrote:

If it was from tooling, it must have stopped with the first case it
found, as is often the case. I just tried myself and found three more
apparent bugs with EXCLUDE both with and without the v1 patch. Do you
want to see the reproducers, or rethink the risk/reward ratio of
what's handled? I'm also not sure how common that clause is.

Please share what you discovered. It would be good to understand if
it's just a simple missing check that can be added to handle all
three, or if there's just more complexity in this than making this
work correctly is worth.

I was going to share the three examples, but then I had a better idea:
I had Claude generate a PL/pgSQL script to exhaustively (or something
close to it) iterate through the the whole space on a few different
data set shapes, look for plans with run conditions, and compare the
result to the "c+0" oracle. (attached as
count_runcondition_matrix.sql). On master, it found 295 (!) distinct
window specs that give wrong answers with run conditions on at least
one data set (attached as runcond-failures-master.txt). With v2, that
number falls to 162, which is still a surprisingly large number
(attached as runcond-failures-v2.txt). This script takes a while to
run, so it's probably not a good basis for a regression test, but it
seems useful to test against, and it can be modified to test
hypotheses.

I then had Claude generate two patches against master, attached as v3.
One is the minimal fix to get the above test matrix to pass. I didn't
try to determine how many currently working cases got lost that way.
The other is an attempt to provide complete coverage of all
monotonicity proofs. The latter is quite an indigestible mess, so we
probably cannot commit something like that, but I think a reasonable
takeaway is that EXCLUDE presents a really difficult set of cases.

I'll also note that two of v2's tests are valid on the data they work
on, but are not valid in general. I've attached v2-wrong-test-repo.sql
to demonstrate.

--
John Naylor
Amazon Web Services

Hi,

On Jul 5, 2026, at 18:44, John Naylor <johncnaylorls@gmail.com> wrote:

I was going to share the three examples, but then I had a better idea:
I had Claude generate a PL/pgSQL script to exhaustively (or something
close to it) iterate through the the whole space on a few different
data set shapes, look for plans with run conditions, and compare the
result to the "c+0" oracle. (attached as
count_runcondition_matrix.sql). On master, it found 295 (!) distinct
window specs that give wrong answers with run conditions on at least
one data set (attached as runcond-failures-master.txt). With v2, that
number falls to 162, which is still a surprisingly large number
(attached as runcond-failures-v2.txt). This script takes a while to
run, so it's probably not a good basis for a regression test, but it
seems useful to test against, and it can be modified to test
hypotheses.

I then had Claude generate two patches against master, attached as v3.
One is the minimal fix to get the above test matrix to pass. I didn't
try to determine how many currently working cases got lost that way.
The other is an attempt to provide complete coverage of all
monotonicity proofs. The latter is quite an indigestible mess, so we
probably cannot commit something like that, but I think a reasonable
takeaway is that EXCLUDE presents a really difficult set of cases.

I'll also note that two of v2's tests are valid on the data they work
on, but are not valid in general. I've attached v2-wrong-test-repo.sql
to demonstrate.

Thanks for putting together the matrix script. That seems like a very
good way to test hypotheses here.

I have also been thinking about this tradeoff while trying to put
together a patch for it. It looks possible to prove more EXCLUDE cases,
but the code and the test space seem to get complicated quickly. My
current inclination is to be conservative here: return
`MONOTONICFUNC_NONE` unless a case is clearly proven safe. The matrix
results seem to support that direction. That may miss some
run-condition opportunities, but it avoids relying on a monotonicity
claim that is not yet fully justified.

This is also close to why the v1 patch I posted earlier simply disabled
the EXCLUDE cases. That version missed the no-ORDER-BY issue, but the
conservative part matches the direction of your minimal v3: do not try
to infer monotonicity for excluded frames unless the case is very
simple.

One extra axis I noticed in the matrix is FILTER. It currently
enumerates `count(*)` and `count(e)`, but not `count(...) FILTER (...)`,
for example:

```
count(*) FILTER (WHERE ...)
```

That is not the same as plain `count(*)`, and it makes the set of
combinations even larger.

I was about to send a WIP patch that is very close to your minimal v3.
The main code difference is this guard:

```
int exclusion = frameOptions & FRAMEOPTION_EXCLUSION;
bool plain_count_star = req->window_func->winfnoid == F_COUNT_ &&
req->window_func->aggfilter == NULL;

if (exclusion &&
(exclusion != FRAMEOPTION_EXCLUDE_CURRENT_ROW || !plain_count_star))
{
req->monotonic = MONOTONICFUNC_NONE;
PG_RETURN_POINTER(req);
}
```

So compared with the minimal v3 rule that rejects all EXCLUDE cases,
this only keeps plain `count(*) EXCLUDE CURRENT ROW`, with no FILTER.
The reason is that removing the current row removes a fixed
contribution. Everything else with EXCLUDE returns
`MONOTONICFUNC_NONE`. I am not strongly attached to keeping that case
if people prefer the simpler minimal fix, but I think the general shape
should still be reject-by-default rather than allow-by-default.

So I agree with the takeaway from your complete-coverage experiment:
there are valid optimizations in this area, but proving all of them
seems too complex for a bug fix. Starting from the minimal safe rule
and adding back only small, clearly proven cases seems safer to me.

--
Best regards,
Chengpeng Yan

On Mon, 6 Jul 2026 at 00:54, Chengpeng Yan <chengpeng_yan@outlook.com> wrote:

count(*) FILTER (WHERE ...)

Yeah, looks like we'll need to disable if there's a FILTER clause.

int exclusion = frameOptions & FRAMEOPTION_EXCLUSION;
bool plain_count_star = req->window_func->winfnoid == F_COUNT_ &&
req->window_func->aggfilter == NULL;

if (exclusion &&
(exclusion != FRAMEOPTION_EXCLUDE_CURRENT_ROW || !plain_count_star))
{
req->monotonic = MONOTONICFUNC_NONE;
PG_RETURN_POINTER(req);
}
```

Given how complex Claude's logic became to get all those cases
correct, I now think the above is the best option. I've attached a
patch for that.

David

Hi David,

On Jul 6, 2026, at 15:24, David Rowley <dgrowleyml@gmail.com> wrote:`

Given how complex Claude's logic became to get all those cases
correct, I now think the above is the best option. I've attached a
patch for that.

Thanks for updating the patch. I looked through the code and it looks
good to me.

One minor comment: this test comment seems to be a typo:

-- As above, but check we detect it's monotonically increasing

It looks like the following test is checking that we do not treat the
frame as monotonically increasing.

Other than that, LGTM.

--
Best regards,
Chengpeng Yan

On Mon, Jul 6, 2026 at 2:25 PM David Rowley <dgrowleyml@gmail.com> wrote:

Given how complex Claude's logic became to get all those cases
correct, I now think the above is the best option. I've attached a
patch for that.

Works for me. A tiny question on this comment change:

- /* No ORDER BY clause then all rows are peers */
...
+ /* No ORDER BY clause and RANGE mode means all rows are peers. */

Was the old comment wrong? Or are all rows still peers, but that's not
sufficient to show monotonicity?

--
John Naylor
Amazon Web Services

On Tue, 7 Jul 2026 at 17:11, John Naylor <johncnaylorls@gmail.com> wrote:

- /* No ORDER BY clause then all rows are peers */
...
+ /* No ORDER BY clause and RANGE mode means all rows are peers. */

Was the old comment wrong? Or are all rows still peers, but that's not
sufficient to show monotonicity?

The test needed to be updated because detecting monotonically
increasing and decreasing in ROWS mode wasn't correct. The comment got
updated along with the test.

David

On Tue, 7 Jul 2026 at 17:11, John Naylor <johncnaylorls@gmail.com> wrote:

Works for me.

Pushed.

David