On 04/09/2026 10:55, David Rowley wrote:
> On Fri, 4 Sept 2026 at 16:59, Michael Paquier <[email protected]> wrote:
>> Any thoughts or comments from others?
> The patch adds PG_RETURN_INPUT(n) to avoid some of the branching in
> int4_sum() so that it immediately returns the aggstate when the value
> being aggregated is null. With my compiler, it cut int4_sum from 18
> down to 16 instructions.
Thanks for your attention.
I adopted your changes. Although I don't see any overhead beyond noise, it seems
better to optimise than to keep it as is.
Also, tests were rewritten - instead of a direct call, I have used the initcond
trick.
--
regards, Andrei Lepikhov,
pgEdge
From ef13930f01897f8411a0ed12cbf99a1023241616 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
Date: Fri, 4 Sep 2026 13:25:40 +0200
Subject: [PATCH v1] 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
