Horton

Thread

Commits

  1. Fix integer overflow in nodeWindowAgg.c

  1. BUG #19405: Assertion in eval_windowaggregates() fails due to integer overflow

    The Post Office <noreply@postgresql.org> — 2026-02-13T05:00:01Z

    The following bug has been logged on the website:

Bug reference:      19405
Logged by:          Alexander Lakhin
Email address:      exclusion@gmail.com
PostgreSQL version: 18.2
Operating system:   Ubuntu 24.04
Description:        

The following script:
CREATE TABLE t (i integer);
INSERT INTO t SELECT g FROM generate_series(1, 2) g;
SELECT SUM(i) OVER (ROWS BETWEEN 1 PRECEDING AND 0x7fffffffffffffff
FOLLOWING EXCLUDE CURRENT ROW) FROM t;

triggers:
TRAP: failed Assert("aggregatedupto_nonrestarted <=
winstate->aggregatedupto"), File: "nodeWindowAgg.c", Line: 1024, PID:
3288248
ExceptionalCondition at assert.c:51:13
eval_windowaggregates at nodeWindowAgg.c:1061:31
ExecWindowAgg at nodeWindowAgg.c:2367:5
ExecProcNode at executor.h:320:1
ExecutePlan at execMain.c:1711:10
standard_ExecutorRun at execMain.c:366:3
...

Reproduced on REL_14_STABLE .. master.

  2. Re: BUG #19405: Assertion in eval_windowaggregates() fails due to integer overflow

    Richard Guo <guofenglinux@gmail.com> — 2026-02-14T09:41:00Z

    On Fri, Feb 13, 2026 at 7:09 PM PG Bug reporting form
<noreply@postgresql.org> wrote:
> The following script:
> CREATE TABLE t (i integer);
> INSERT INTO t SELECT g FROM generate_series(1, 2) g;
> SELECT SUM(i) OVER (ROWS BETWEEN 1 PRECEDING AND 0x7fffffffffffffff
> FOLLOWING EXCLUDE CURRENT ROW) FROM t;

Thanks for the report.  Reproduced here.

It seems to be caused by a signed integer overflow in row_is_in_frame
when calculating the frame's end position:

            if (pos > winstate->currentpos + offset)
                return -1;

When offset is very large (close to INT64_MAX, as in the reported
case), the addition can overflow, in which case the result would wrap
to a negative number (with -fwrapv), causing the comparison to
incorrectly return true.  In release builds, this causes valid rows to
be excluded from the window frame.  In debug builds, it leads to an
assertion failure.

I think we can fix this by leveraging the overflow-aware integer
operation (ie, pg_add_s64_overflow) to perform the addition here.  If
an overflow is detected, we can assume the frame boundary extends to
the end of the partition, meaning the current row is within the frame.

- Richard

  3. Re: BUG #19405: Assertion in eval_windowaggregates() fails due to integer overflow

    Alexander Law <exclusion@gmail.com> — 2026-02-14T10:00:00Z

    Hello Richard,

14.02.2026 11:41, Richard Guo wrote:
> On Fri, Feb 13, 2026 at 7:09 PM PG Bug reporting form
> <noreply@postgresql.org> wrote:
>> The following script:
>> CREATE TABLE t (i integer);
>> INSERT INTO t SELECT g FROM generate_series(1, 2) g;
>> SELECT SUM(i) OVER (ROWS BETWEEN 1 PRECEDING AND 0x7fffffffffffffff
>> FOLLOWING EXCLUDE CURRENT ROW) FROM t;
> Thanks for the report.  Reproduced here.
>
> It seems to be caused by a signed integer overflow in row_is_in_frame
> when calculating the frame's end position:
>
>              if (pos > winstate->currentpos + offset)
>                  return -1;
>
> When offset is very large (close to INT64_MAX, as in the reported
> case), the addition can overflow, in which case the result would wrap
> to a negative number (with -fwrapv), causing the comparison to
> incorrectly return true.  In release builds, this causes valid rows to
> be excluded from the window frame.  In debug builds, it leads to an
> assertion failure.
>
> I think we can fix this by leveraging the overflow-aware integer
> operation (ie, pg_add_s64_overflow) to perform the addition here.  If
> an overflow is detected, we can assume the frame boundary extends to
> the end of the partition, meaning the current row is within the frame.

Thank you for looking at this!

The patch works for me, but I've just discovered one more similar anomaly:
SELECT SUM(i) OVER (ROWS BETWEEN 0x7fffffffffffffff FOLLOWING AND 1 FOLLOWING), i FROM t;

ERROR:  XX000: window frame head moved backward
LOCATION:  eval_windowaggregates, nodeWindowAgg.c:782

Best regards,
Alexander

  4. Re: BUG #19405: Assertion in eval_windowaggregates() fails due to integer overflow

    Richard Guo <guofenglinux@gmail.com> — 2026-02-14T11:00:51Z

    On Sat, Feb 14, 2026 at 7:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> The patch works for me, but I've just discovered one more similar anomaly:
> SELECT SUM(i) OVER (ROWS BETWEEN 0x7fffffffffffffff FOLLOWING AND 1 FOLLOWING), i FROM t;
>
> ERROR:  XX000: window frame head moved backward
> LOCATION:  eval_windowaggregates, nodeWindowAgg.c:782

Right, I noticed this one too.  Basically, nodeWindowAgg.c doesn't
check for overflow when adding startOffsetValue or endOffsetValue.
Since these values are provided by the user and can be arbitrarily
large, simple addition does not seem safe.  I think we may need to
switch to overflow-aware integer operations in all relevant code.

- Richard

  5. Re: BUG #19405: Assertion in eval_windowaggregates() fails due to integer overflow

    Richard Guo <guofenglinux@gmail.com> — 2026-02-14T13:44:48Z

    On Sat, Feb 14, 2026 at 8:00 PM Richard Guo <guofenglinux@gmail.com> wrote:
> Right, I noticed this one too.  Basically, nodeWindowAgg.c doesn't
> check for overflow when adding startOffsetValue or endOffsetValue.
> Since these values are provided by the user and can be arbitrarily
> large, simple addition does not seem safe.  I think we may need to
> switch to overflow-aware integer operations in all relevant code.

Here is an updated patch to fix all relevant code in nodeWindowAgg.c.

- Richard

  6. Re: BUG #19405: Assertion in eval_windowaggregates() fails due to integer overflow

    Tender Wang <tndrwang@gmail.com> — 2026-02-15T08:48:04Z

    Richard Guo <guofenglinux@gmail.com> 于2026年2月14日周六 17:41写道：
>
> On Fri, Feb 13, 2026 at 7:09 PM PG Bug reporting form
> <noreply@postgresql.org> wrote:
> > The following script:
> > CREATE TABLE t (i integer);
> > INSERT INTO t SELECT g FROM generate_series(1, 2) g;
> > SELECT SUM(i) OVER (ROWS BETWEEN 1 PRECEDING AND 0x7fffffffffffffff
> > FOLLOWING EXCLUDE CURRENT ROW) FROM t;
>
> Thanks for the report.  Reproduced here.
>
> It seems to be caused by a signed integer overflow in row_is_in_frame
> when calculating the frame's end position:
>
>             if (pos > winstate->currentpos + offset)
>                 return -1;
>
> When offset is very large (close to INT64_MAX, as in the reported
> case), the addition can overflow, in which case the result would wrap
> to a negative number (with -fwrapv), causing the comparison to
> incorrectly return true.  In release builds, this causes valid rows to
> be excluded from the window frame.  In debug builds, it leads to an
> assertion failure.

Yes, the code above may overflow; in debug builds, the assertion would fail.

>
> I think we can fix this by leveraging the overflow-aware integer
> operation (ie, pg_add_s64_overflow) to perform the addition here.  If
> an overflow is detected, we can assume the frame boundary extends to
> the end of the partition, meaning the current row is within the frame.
>
I've also considered similar solutions.  But I'm not very familiar
with the window function
internal codes, so not sure it's the right fix.


> Right, I noticed this one too.  Basically, nodeWindowAgg.c doesn't
> check for overflow when adding startOffsetValue or endOffsetValue.
> Since these values are provided by the user and can be arbitrarily
> large, simple addition does not seem safe.  I think we may need to
> switch to overflow-aware integer operations in all relevant code.

>Here is an updated patch to fix all relevant code in nodeWindowAgg.c.

v2 seems to cover all cases. WFM.

In window.sql, we don't have a test case for this issue. I think we
should add it to the window.sql

-- 
Thanks,
Tender Wang

  7. Re: BUG #19405: Assertion in eval_windowaggregates() fails due to integer overflow

    Richard Guo <guofenglinux@gmail.com> — 2026-02-17T01:55:30Z

    On Sun, Feb 15, 2026 at 5:48 PM Tender Wang <tndrwang@gmail.com> wrote:
> v2 seems to cover all cases. WFM.
>
> In window.sql, we don't have a test case for this issue. I think we
> should add it to the window.sql

I've included test cases covering the overflow scenarios for ROWS mode
in v3.  (I failed to come up with queries for GROUPS mode that
demonstrate the bug, but I suspect I just haven't found the right test
case yet.)  I have also included a commit message.

- Richard

  8. Re: BUG #19405: Assertion in eval_windowaggregates() fails due to integer overflow

    Richard Guo <guofenglinux@gmail.com> — 2026-04-08T09:42:12Z

    On Tue, Feb 17, 2026 at 10:55 AM Richard Guo <guofenglinux@gmail.com> wrote:
> I've included test cases covering the overflow scenarios for ROWS mode
> in v3.  (I failed to come up with queries for GROUPS mode that
> demonstrate the bug, but I suspect I just haven't found the right test
> case yet.)  I have also included a commit message.

Here is the updated patch.  I've added test cases for GROUPS mode with
an overflow-inducing offset.  These don't seem to produce visibly
wrong results without the patch.  I believe that is because the
incremental nature of group pointer advancement happens to mask the
misbehavior.  But I think they are still worth having since signed
integer overflow is undefined behavior in C.

Regarding performance, I don't think this should be a concern.
pg_add_s64_overflow compiles down to a native add+jo sequence on
platforms with __builtin_add_overflow, and falls back to a couple of
inlined comparisons on others.  Either way, the cost seems negligible
next to the per-row tuple store and aggregation work these functions
already do.

- Richard

  9. Re: BUG #19405: Assertion in eval_windowaggregates() fails due to integer overflow

    Richard Guo <guofenglinux@gmail.com> — 2026-04-10T09:24:14Z

    On Wed, Apr 8, 2026 at 6:42 PM Richard Guo <guofenglinux@gmail.com> wrote:
> Here is the updated patch.  I've added test cases for GROUPS mode with
> an overflow-inducing offset.  These don't seem to produce visibly
> wrong results without the patch.  I believe that is because the
> incremental nature of group pointer advancement happens to mask the
> misbehavior.  But I think they are still worth having since signed
> integer overflow is undefined behavior in C.
>
> Regarding performance, I don't think this should be a concern.
> pg_add_s64_overflow compiles down to a native add+jo sequence on
> platforms with __builtin_add_overflow, and falls back to a couple of
> inlined comparisons on others.  Either way, the cost seems negligible
> next to the per-row tuple store and aggregation work these functions
> already do.

I forgot to mention that I've committed and back-patched this fix.

- Richard