| From: | Andrei Lepikhov <lepihov(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, David Rowley <dgrowleyml(at)gmail(dot)com> |
| Subject: | Re: Make the transition state of avg(int2)/avg(int4)/sum(int2)/sum(int4) internal |
| Date: | 2026-09-07 14:28:16 |
| Message-ID: | 2c61c2c2-6bf3-4155-b569-713ddbb5d22c@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 04/09/2026 17:50, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
>> Seems like if we do this - and I think we should - we should go broader than
>> just doing this for int8[]. So yea, let's do it for float8 too.
>> I think it might make sense to have an opr_sanity.sql check that verifies that
>> we don't add new builtin aggregates that have an array transition state. Or
>> perhaps even more strictly, test that aggregates either have an internal
>> transition state, or the argument type's (for stuff like min/max).
>
> On reflection, I'm not sure that such a policy is a win. If you go
> with an internal-type transition state, then (if you want parallel
> aggregation support) you need serialize/deserialize functions, and
> I think also some other stuff that comes for free if the transition
> state is a real SQL type. So this is not so much a clear win as
> a tradeoff of which code you want to write.
>
As I see it, with an internal state, some aggregates gain, while others don't.
Plain sum(int2/int4/float8) already has a trivial 8-byte transition state:
minimal memory, passed by value, no detoast. The moving-aggregate mode of
sum(int2/int4), on the other hand, reuses avg()'s int2/int4_avg_accum_inv
functions, which pay for array overhead and a detoast check on every row.
Internal state might reduce memory consumption there.
float8 has no moving-aggregate mode. Also note that the regression tests
currently call float8_regr_combine() directly, with literal array values.
Not sure how much it matters, but introducing an internal state also means
giving up initcond. Hence, sum(float8) with an internal state would lose its
STRICT marking, pushing NULL-checking into the function body.
Quick tests show sum() losing 1-3% in serial mode and a more significant ~7% in
a parallel GROUP BY, due to serialize/deserialize across workers. This was run
on my laptop, not a dedicated benchmarking box, but the overhead looks clearly
detectable.
Overall, in my opinion, only AVG() and other statistical aggregates, such as
STDDEV(), benefit from moving to internal state. Basic aggregates like sum()
should probably stay as they are, if we want to avoid a performance regression.
--
regards, Andrei Lepikhov,
pgEdge
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jan Nidzwetzki | 2026-09-07 14:39:29 | Prevent object capture in CREATE/ALTER EXTENSION scripts |
| Previous Message | Andrey Kazarinov | 2026-09-07 14:26:29 | [PATCH] Allow subquery pull-up past inlineable CTEs |