| From: | Andres Freund <andres(at)anarazel(dot)de> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | Andrei Lepikhov <lepihov(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Subject: | Re: SUM(int2)/SUM(int4) do not detect overflow of the int8 accumulator |
| Date: | 2026-09-02 23:56:32 |
| Message-ID: | lbvjl22swi3p3sf7nmmrlx2cq5aa55ry3c5ore2rpylsmi6s5j@xrrahkipnsce |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On 2026-09-02 13:42:49 +0800, Chao Li wrote:
> I agree that this may not happen during normal aggregate execution, but int2_avg_accum_inv() is also directly callable as a SQL function. For example:
> ```
> evantest=# select int2_avg_accum_inv('{1,9223372036854775807}'::int8[], -1::int2);
> int2_avg_accum_inv
> --------------------------
> {0,-9223372036854775808}
> (1 row)
> ```
>
> It’s showing an overflow for a caller-supplied state. I wouldn’t submit a
> dedicated patch for this case, but I just thought that, since this patch
> adds overflow checks to the neighboring transition functions, perhaps it
> would be worth handling this one at the same time. But, anyway, that’s not a
> strong comment, it’s up to you.
FWIW, I think we should seriously consider making [almost] all the transition
states internal. Having to support calling these functions in non-aggregate
contexts adds complexity without any actual gain. We shouldn't need to check
whether we are in an AggContext, whether the argument is toasted, whether
there are NULL elements in the array, compute offsets into the array, etc.
Greetings,
Andres Freund
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-09-03 00:10:33 | Re: SUM(int2)/SUM(int4) do not detect overflow of the int8 accumulator |
| Previous Message | Michael Paquier | 2026-09-02 23:44:12 | Re: SUM(int2)/SUM(int4) do not detect overflow of the int8 accumulator |