On 07/09/2026 10:25, Andrei Lepikhov wrote:
> 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.
It looks like I mistakenly sent the old version as v2. Here is the actual
version (v3) of the patch.

-- 
regards, Andrei Lepikhov,
pgEdge
From dff93c60e520e6f42dc05a1d78cc63c60b11c3d6 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
Date: Fri, 4 Sep 2026 13:25:40 +0200
Subject: [PATCH v3] 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 | 66 +++++++++++++++++++++------------
 src/include/fmgr.h              | 11 ++++++
 2 files changed, 54 insertions(+), 23 deletions(-)

diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 37f24e33857..cf76050c3a1 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -6350,24 +6350,24 @@ int2_sum(PG_FUNCTION_ARGS)
        int64           oldsum;
        int64           newval;
 
+       /* Return the left input unchanged if right input is null. */
+       if (PG_ARGISNULL(1))
+               PG_RETURN_INPUT(0);
+
        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 +6378,24 @@ int4_sum(PG_FUNCTION_ARGS)
        int64           oldsum;
        int64           newval;
 
+       /* Return the left input unchanged if right input is null. */
+       if (PG_ARGISNULL(1))
+               PG_RETURN_INPUT(0);
+
        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 +6457,7 @@ int2_avg_accum(PG_FUNCTION_ARGS)
        ArrayType  *transarray;
        int16           newval = PG_GETARG_INT16(1);
        Int8TransTypeData *transdata;
+       int64           newsum;
 
        /*
         * If we're invoked as an aggregate, we can cheat and modify our first
@@ -6473,8 +6474,14 @@ int2_avg_accum(PG_FUNCTION_ARGS)
                elog(ERROR, "expected 2-element int8 array");
 
        transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray);
+
+       if (unlikely(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++;
-       transdata->sum += newval;
+       transdata->sum = newsum;
 
        PG_RETURN_ARRAYTYPE_P(transarray);
 }
@@ -6485,6 +6492,7 @@ int4_avg_accum(PG_FUNCTION_ARGS)
        ArrayType  *transarray;
        int32           newval = PG_GETARG_INT32(1);
        Int8TransTypeData *transdata;
+       int64           newsum;
 
        /*
         * If we're invoked as an aggregate, we can cheat and modify our first
@@ -6501,8 +6509,14 @@ int4_avg_accum(PG_FUNCTION_ARGS)
                elog(ERROR, "expected 2-element int8 array");
 
        transdata = (Int8TransTypeData *) ARR_DATA_PTR(transarray);
+
+       if (unlikely(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++;
-       transdata->sum += newval;
+       transdata->sum = newsum;
 
        PG_RETURN_ARRAYTYPE_P(transarray);
 }
@@ -6514,6 +6528,7 @@ int4_avg_combine(PG_FUNCTION_ARGS)
        ArrayType  *transarray2;
        Int8TransTypeData *state1;
        Int8TransTypeData *state2;
+       int64           newsum;
 
        if (!AggCheckCallContext(fcinfo, NULL))
                elog(ERROR, "aggregate function called in non-aggregate 
context");
@@ -6532,8 +6547,13 @@ int4_avg_combine(PG_FUNCTION_ARGS)
        state1 = (Int8TransTypeData *) ARR_DATA_PTR(transarray1);
        state2 = (Int8TransTypeData *) ARR_DATA_PTR(transarray2);
 
+       if (unlikely(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 += state2->count;
-       state1->sum += state2->sum;
+       state1->sum = newsum;
 
        PG_RETURN_ARRAYTYPE_P(transarray1);
 }
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index 04b7914095f..8db5221e1dd 100644
--- a/src/include/fmgr.h
+++ b/src/include/fmgr.h
@@ -349,6 +349,17 @@ 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)
-- 
2.55.0

Reply via email to