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