From dff93c60e520e6f42dc05a1d78cc63c60b11c3d6 Mon Sep 17 00:00:00 2001 From: "Andrei V. Lepikhov" 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