On 05/09/2026 19:01, Tom Lane wrote:
> So in these bits:
>
> + /* This is the first non-null input. */
> if (PG_ARGISNULL(0))
> - {
> - /* No non-null input seen so far... */
>
> the replacement comment is badly placed.
Fixed. I'm still not entirely sure where one-line comments like this should go —
I've seen them both right before the if and inside the block. It's probably
obvious to native speakers, but it leaves me a bit confused.
>
> At a less nit-picky level:
>
> * In the avg_accum functions, we can argue about how likely it
> is that we'd reach overflow of the "sum" fields, but it is completely
> insane to expend cycles and code complexity to check for overflow of
> the "count" fields. If you can reach 2^63 by repeated addition of 1
> within the lifetime of a PG database, then we have got far worse
> problems, eg with WAL LSN overflow.
Ok, fixed.
>
> * PG_RETURN_INPUT is not laid out per our usual conventions.
> If you need a do/while wrapper, start it on the next line.
Done
> * I would not include one single one of these test cases.
> They are not worth the development effort nor the forevermore
> test runtime cost, especially since they are testing faked-up
> scenarios.
Removed. Adding a regression test alongside a bug fix is just my default habit,
so I followed it here without thinking it through - happy to drop them.
--
regards, Andrei Lepikhov,
pgEdge
From 71a0814a42ef364f3737d38af84c639fb0ec64a4 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
Date: Fri, 4 Sep 2026 13:25:40 +0200
Subject: [PATCH v2] Detect overflow of the int8 accumulator in sum() and avg()
over int2/int4
sum(int2) and sum(int4) accumulate into an int8, and avg(int2)/avg(int4)
into a two-element int8 array holding count and sum, on the assumption
that an int8 accumulator is wide enough for any practical number of
narrower inputs. It is not quite wide enough for the modern tables.
So, hitting the limit the aggregate silently wraps around.
For sum(int4) the answer even depended on the plan shape. Partial
aggregates are combined with int8pl, which does check for overflow, so
the same query over the same data returned a wrapped negative number
under a serial plan and failed with "bigint out of range" under parallel
aggregation. avg() was wrong under every plan, as int4_avg_accum and
int4_avg_combine both added into the state unchecked.
Check every addition and subtraction in int2_sum, int4_sum, int{2,4}_avg_accum,
and int4_avg_combine with pg_add_s64_overflow/pg_sub_s64_overflow, and report
the usual "bigint out of range" error instead of wrapping. The count field of
the avg() state is checked as well: counting rows cannot realistically overflow
it, but that state is an ordinary SQL array, so a custom aggregate can start
from any initial condition it likes.
---
src/backend/utils/adt/numeric.c | 94 ++++++++++++++++--------
src/include/fmgr.h | 10 +++
src/test/regress/expected/aggregates.out | 45 ++++++++++++
src/test/regress/sql/aggregates.sql | 32 ++++++++
4 files changed, 149 insertions(+), 32 deletions(-)
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 37f24e33857..ba87726abbc 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -6350,24 +6350,26 @@ int2_sum(PG_FUNCTION_ARGS)
int64 oldsum;
int64 newval;
+ /*
+ * Return the running sum unchanged if the new input is null. This also
+ * covers the case where no non-null input has been seen yet, as the
+ * running sum is null then too.
+ */
+ if (PG_ARGISNULL(1))
+ PG_RETURN_INPUT(0);
+
+ /* This is the first non-null input. */
if (PG_ARGISNULL(0))
- {
- /* No non-null input seen so far... */
- if (PG_ARGISNULL(1))
- PG_RETURN_NULL(); /* still no non-null */
- /* This is the first non-null input. */
- newval = (int64) PG_GETARG_INT16(1);
- PG_RETURN_INT64(newval);
- }
+ PG_RETURN_INT64((int64) PG_GETARG_INT16(1));
oldsum = PG_GETARG_INT64(0);
- /* Leave sum unchanged if new input is null. */
- if (PG_ARGISNULL(1))
- PG_RETURN_INT64(oldsum);
-
/* OK to do the addition. */
- newval = oldsum + (int64) PG_GETARG_INT16(1);
+ if (unlikely(pg_add_s64_overflow(oldsum, (int64) PG_GETARG_INT16(1),
+
&newval)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));
PG_RETURN_INT64(newval);
}
@@ -6378,24 +6380,26 @@ int4_sum(PG_FUNCTION_ARGS)
int64 oldsum;
int64 newval;
+ /*
+ * Return the running sum unchanged if the new input is null. This also
+ * covers the case where no non-null input has been seen yet, as the
+ * running sum is null then too.
+ */
+ if (PG_ARGISNULL(1))
+ PG_RETURN_INPUT(0);
+
+ /* This is the first non-null input. */
if (PG_ARGISNULL(0))
- {
- /* No non-null input seen so far... */
- if (PG_ARGISNULL(1))
- PG_RETURN_NULL(); /* still no non-null */
- /* This is the first non-null input. */
- newval = (int64) PG_GETARG_INT32(1);
- PG_RETURN_INT64(newval);
- }
+ PG_RETURN_INT64((int64) PG_GETARG_INT32(1));
oldsum = PG_GETARG_INT64(0);
- /* Leave sum unchanged if new input is null. */
- if (PG_ARGISNULL(1))
- PG_RETURN_INT64(oldsum);
-
/* OK to do the addition. */
- newval = oldsum + (int64) PG_GETARG_INT32(1);
+ if (unlikely(pg_add_s64_overflow(oldsum, (int64) PG_GETARG_INT32(1),
+
&newval)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));
PG_RETURN_INT64(newval);
}
@@ -6457,6 +6461,8 @@ int2_avg_accum(PG_FUNCTION_ARGS)
ArrayType *transarray;
int16 newval = PG_GETARG_INT16(1);
Int8TransTypeData *transdata;
+ int64 newcount;
+ int64 newsum;
/*
* If we're invoked as an aggregate, we can cheat and modify our first
@@ -6473,8 +6479,15 @@ int2_avg_accum(PG_FUNCTION_ARGS)
elog(ERROR, "expected 2-element int8 array");
transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray);
- transdata->count++;
- transdata->sum += newval;
+
+ if (unlikely(pg_add_s64_overflow(transdata->count, 1, &newcount) ||
+ pg_add_s64_overflow(transdata->sum, (int64)
newval, &newsum)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));
+
+ transdata->count = newcount;
+ transdata->sum = newsum;
PG_RETURN_ARRAYTYPE_P(transarray);
}
@@ -6485,6 +6498,8 @@ int4_avg_accum(PG_FUNCTION_ARGS)
ArrayType *transarray;
int32 newval = PG_GETARG_INT32(1);
Int8TransTypeData *transdata;
+ int64 newcount;
+ int64 newsum;
/*
* If we're invoked as an aggregate, we can cheat and modify our first
@@ -6501,8 +6516,15 @@ int4_avg_accum(PG_FUNCTION_ARGS)
elog(ERROR, "expected 2-element int8 array");
transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray);
- transdata->count++;
- transdata->sum += newval;
+
+ if (unlikely(pg_add_s64_overflow(transdata->count, 1, &newcount) ||
+ pg_add_s64_overflow(transdata->sum, (int64)
newval, &newsum)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));
+
+ transdata->count = newcount;
+ transdata->sum = newsum;
PG_RETURN_ARRAYTYPE_P(transarray);
}
@@ -6514,6 +6536,8 @@ int4_avg_combine(PG_FUNCTION_ARGS)
ArrayType *transarray2;
Int8TransTypeData *state1;
Int8TransTypeData *state2;
+ int64 newcount;
+ int64 newsum;
if (!AggCheckCallContext(fcinfo, NULL))
elog(ERROR, "aggregate function called in non-aggregate
context");
@@ -6532,8 +6556,14 @@ int4_avg_combine(PG_FUNCTION_ARGS)
state1 = (Int8TransTypeData *) ARR_DATA_PTR(transarray1);
state2 = (Int8TransTypeData *) ARR_DATA_PTR(transarray2);
- state1->count += state2->count;
- state1->sum += state2->sum;
+ if (unlikely(pg_add_s64_overflow(state1->count, state2->count,
&newcount) ||
+ pg_add_s64_overflow(state1->sum, state2->sum,
&newsum)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));
+
+ state1->count = newcount;
+ state1->sum = newsum;
PG_RETURN_ARRAYTYPE_P(transarray1);
}
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index 04b7914095f..fb767700df7 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -349,6 +349,16 @@ extern varlena *pg_detoast_datum_packed(varlena *datum);
/* A few internal functions return void (which is not the same as NULL!) */
#define PG_RETURN_VOID() return (Datum) 0
+/*
+ * A shortcut to allow functions to return the value of the given input
+ * parameter, NULL if that parameter was NULL and the value of the parameter
+ * otherwise. The caller is responsible for ensuring the types match.
+ */
+#define PG_RETURN_INPUT(n) do { \
+ fcinfo->isnull = fcinfo->args[n].isnull; \
+ return fcinfo->args[n].value; \
+ } while (0)
+
/* Macros for returning results of standard types */
#define PG_RETURN_DATUM(x) return (x)
diff --git a/src/test/regress/expected/aggregates.out
b/src/test/regress/expected/aggregates.out
index 7d07619956f..f7089a0f8b3 100644
--- a/src/test/regress/expected/aggregates.out
+++ b/src/test/regress/expected/aggregates.out
@@ -821,6 +821,51 @@ SELECT sum(q1*2000), sum(-q1*2000), 2000*sum(q1) FROM
int8_tbl;
27407340740741226000 | -27407340740741226000 | 27407340740741226000
(1 row)
+--
+-- sum(int2)/sum(int4) and avg(int2)/avg(int4) accumulate into int8, and that
+-- accumulator must report an overflow rather than silently wrap around.
+-- Summing enough real rows to reach the int8 limit would take billions of
+-- them, so use custom aggregates over the same transition functions with an
+-- initial condition that already sits next to the limit.
+--
+CREATE AGGREGATE ovf_sum_int4 (int4) (
+ sfunc = int4_sum, stype = int8, initcond = '9223372036854775806'
+);
+SELECT ovf_sum_int4(v) FROM (VALUES (1)) t(v);
+ ovf_sum_int4
+---------------------
+ 9223372036854775807
+(1 row)
+
+SELECT ovf_sum_int4(v) FROM (VALUES (1), (NULL)) t(v);
+ ovf_sum_int4
+---------------------
+ 9223372036854775807
+(1 row)
+
+SELECT ovf_sum_int4(v) FROM (VALUES (1), (1)) t(v); -- ERROR
+ERROR: bigint out of range
+CREATE AGGREGATE ovf_sum_int2 (int2) (
+ sfunc = int2_sum, stype = int8, initcond = '-9223372036854775807'
+);
+SELECT ovf_sum_int2(v) FROM (VALUES ('-1'::int2), ('-1'::int2)) t(v); -- ERROR
+ERROR: bigint out of range
+-- avg(int2)/avg(int4) keep count and sum in a two-element int8 array
+CREATE AGGREGATE ovf_avg_int4 (int4) (
+ sfunc = int4_avg_accum, stype = _int8, finalfunc = int8_avg,
+ initcond = '{0,9223372036854775806}'
+);
+SELECT ovf_avg_int4(v) FROM (VALUES (1)) t(v);
+ ovf_avg_int4
+---------------------
+ 9223372036854775807
+(1 row)
+
+SELECT ovf_avg_int4(v) FROM (VALUES (1), (1)) t(v); -- ERROR
+ERROR: bigint out of range
+DROP AGGREGATE ovf_sum_int4 (int4);
+DROP AGGREGATE ovf_sum_int2 (int2);
+DROP AGGREGATE ovf_avg_int4 (int4);
-- test for outer-level aggregates
-- this should work
select ten, sum(distinct four) from onek a
diff --git a/src/test/regress/sql/aggregates.sql
b/src/test/regress/sql/aggregates.sql
index 91f8342166f..7adb732a26c 100644
--- a/src/test/regress/sql/aggregates.sql
+++ b/src/test/regress/sql/aggregates.sql
@@ -228,6 +228,38 @@ SELECT sum(q1+q2), sum(q1)+sum(q2) FROM int8_tbl;
SELECT sum(q1-q2), sum(q2-q1), sum(q1)-sum(q2) FROM int8_tbl;
SELECT sum(q1*2000), sum(-q1*2000), 2000*sum(q1) FROM int8_tbl;
+--
+-- sum(int2)/sum(int4) and avg(int2)/avg(int4) accumulate into int8, and that
+-- accumulator must report an overflow rather than silently wrap around.
+-- Summing enough real rows to reach the int8 limit would take billions of
+-- them, so use custom aggregates over the same transition functions with an
+-- initial condition that already sits next to the limit.
+--
+
+CREATE AGGREGATE ovf_sum_int4 (int4) (
+ sfunc = int4_sum, stype = int8, initcond = '9223372036854775806'
+);
+SELECT ovf_sum_int4(v) FROM (VALUES (1)) t(v);
+SELECT ovf_sum_int4(v) FROM (VALUES (1), (NULL)) t(v);
+SELECT ovf_sum_int4(v) FROM (VALUES (1), (1)) t(v); -- ERROR
+
+CREATE AGGREGATE ovf_sum_int2 (int2) (
+ sfunc = int2_sum, stype = int8, initcond = '-9223372036854775807'
+);
+SELECT ovf_sum_int2(v) FROM (VALUES ('-1'::int2), ('-1'::int2)) t(v); -- ERROR
+
+-- avg(int2)/avg(int4) keep count and sum in a two-element int8 array
+CREATE AGGREGATE ovf_avg_int4 (int4) (
+ sfunc = int4_avg_accum, stype = _int8, finalfunc = int8_avg,
+ initcond = '{0,9223372036854775806}'
+);
+SELECT ovf_avg_int4(v) FROM (VALUES (1)) t(v);
+SELECT ovf_avg_int4(v) FROM (VALUES (1), (1)) t(v); -- ERROR
+
+DROP AGGREGATE ovf_sum_int4 (int4);
+DROP AGGREGATE ovf_sum_int2 (int2);
+DROP AGGREGATE ovf_avg_int4 (int4);
+
-- test for outer-level aggregates
-- this should work
--
2.55.0
