From 76d9f5ad76494ce02febfdecef0aa83a8fd8667a Mon Sep 17 00:00:00 2001 From: "Andrei V. Lepikhov" Date: Mon, 31 Aug 2026 15:50:41 +0200 Subject: [PATCH v0] Detect overflow of the int8 accumulator in some aggregate functions The sum(int2) and sum(int4) functions use an int8 value to store their results, since int8 should be large enough for most cases. Until now, the code just added values together without checking for overflow. If overflow occurred, the result would silently wrap around, leading to an incorrect answer. This made the aggregate behave inconsistently. The combine function, int8pl(), has always given a "bigint out of range" error when there is an overflow. The moving-aggregate path uses intX_avg_accum() with avg(), so the same overflow could either cause an error or just wrap around, depending on whether the planner chose a parallel plan. Signed overflow is also undefined behavior, so we cannot trust the wrapped value. One of the main goals of 101c7ee3ee8 was to remove these cases. Now, we check for overflow using pg_add_s64_overflow() and report it in the same way that int8pl() does. The intX_avg_accum() and int4_avg_combine() functions calculate the new sum first, and then update both the count and sum together. This prevents errors that could leave the count and sum out of sync. This change also adds the overflow check to avg(int2) and avg(int4), since they use the same transition functions. --- src/backend/utils/adt/numeric.c | 39 +++++++++++++++++++++--- src/test/regress/expected/aggregates.out | 19 ++++++++++++ src/test/regress/sql/aggregates.sql | 12 ++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 523cd3cd608..a48dab3779c 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -6367,7 +6367,11 @@ int2_sum(PG_FUNCTION_ARGS) 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); } @@ -6395,7 +6399,11 @@ int4_sum(PG_FUNCTION_ARGS) 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 +6465,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 +6482,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 +6500,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 +6517,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 +6536,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 +6555,14 @@ int4_avg_combine(PG_FUNCTION_ARGS) state1 = (Int8TransTypeData *) ARR_DATA_PTR(transarray1); state2 = (Int8TransTypeData *) ARR_DATA_PTR(transarray2); + /* As in intX_avg_accum, don't modify state1 until we know we can. */ + 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/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index 7d07619956f..d33d2f92a15 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -87,6 +87,25 @@ SELECT sum(gpa) AS avg_6_8 FROM ONLY student; 6.8 (1 row) +-- sum(int2) and sum(int4) accumulate into int8. Overflowing that accumulator +-- would take billions of rows, so exercise the transition functions directly. +SELECT int2_sum('9223372036854775807'::int8, 1::int2); +ERROR: bigint out of range +SELECT int4_sum('9223372036854775807'::int8, 1::int4); +ERROR: bigint out of range +SELECT int2_avg_accum('{0,9223372036854775807}'::int8[], 1::int2); +ERROR: bigint out of range +SELECT int4_avg_accum('{0,9223372036854775807}'::int8[], 1::int4); +ERROR: bigint out of range +-- negative inputs must be caught going the other way just the same +SELECT int2_sum('-9223372036854775808'::int8, -1::int2); +ERROR: bigint out of range +SELECT int4_sum('-9223372036854775808'::int8, -1::int4); +ERROR: bigint out of range +SELECT int2_avg_accum('{0,-9223372036854775808}'::int8[], -1::int2); +ERROR: bigint out of range +SELECT int4_avg_accum('{0,-9223372036854775808}'::int8[], -1::int4); +ERROR: bigint out of range SELECT max(four) AS max_3 FROM onek; max_3 ------- diff --git a/src/test/regress/sql/aggregates.sql b/src/test/regress/sql/aggregates.sql index 91f8342166f..264dedd8496 100644 --- a/src/test/regress/sql/aggregates.sql +++ b/src/test/regress/sql/aggregates.sql @@ -42,6 +42,18 @@ SELECT sum(a) AS sum_198 FROM aggtest; SELECT sum(b) AS avg_431_773 FROM aggtest; SELECT sum(gpa) AS avg_6_8 FROM ONLY student; +-- sum(int2) and sum(int4) accumulate into int8. Overflowing that accumulator +-- would take billions of rows, so exercise the transition functions directly. +SELECT int2_sum('9223372036854775807'::int8, 1::int2); +SELECT int4_sum('9223372036854775807'::int8, 1::int4); +SELECT int2_avg_accum('{0,9223372036854775807}'::int8[], 1::int2); +SELECT int4_avg_accum('{0,9223372036854775807}'::int8[], 1::int4); +-- negative inputs must be caught going the other way just the same +SELECT int2_sum('-9223372036854775808'::int8, -1::int2); +SELECT int4_sum('-9223372036854775808'::int8, -1::int4); +SELECT int2_avg_accum('{0,-9223372036854775808}'::int8[], -1::int2); +SELECT int4_avg_accum('{0,-9223372036854775808}'::int8[], -1::int4); + SELECT max(four) AS max_3 FROM onek; SELECT max(a) AS max_100 FROM aggtest; SELECT max(aggtest.b) AS max_324_78 FROM aggtest; -- 2.55.0