On Fri, 4 Sept 2026 at 16:59, Michael Paquier <[email protected]> wrote:
> Any thoughts or comments from others?
I think if we don't add overflow error checking for sum(int2) and
sum(int4) today, we'll need to do it at some point in the future. I
suspect we've only gotten away with it for this long, not because
nobody aggregates 4+ billion rows, but because the values being
aggregated are unlikely to be large enough to cause the overflow.
Since Andrei has demonstrated that it's possible to hit that limit
with a non-parallel query in less than 5 minutes, albeit that is
passing INT_MAX (the most extreme case), it might be worth adding the
checks. I was surprised that it only took 5 mins to do 4 billion rows,
especially with generate_series. It's probably just a matter of time
before someone discovers this with a real-world case out in the wild.
If there's a measurable performance regression from adding the
overflow checks, does the attached buy enough of it back? I couldn't
really measure much of a performance difference from it on my Zen2
machine, so I didn't try with the overflow patch.
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.
David
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 37f24e33857..79295d43ddf 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -6378,22 +6378,15 @@ 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);
diff --git a/src/include/fmgr.h b/src/include/fmgr.h
index 04b7914095f..e3c2b3ac686 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 give 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)
