Re: SUM(int2)/SUM(int4) do not detect overflow of the int8 accumulator

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Andrei Lepikhov <lepihov(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Nathan Bossart <nathandbossart(at)gmail(dot)com>
Subject: Re: SUM(int2)/SUM(int4) do not detect overflow of the int8 accumulator
Date: 2026-09-05 04:51:57
Message-ID: CAApHDvpaJnJ6YNxbLWJ5nWhuKU3bTxt6cKuM=aeEuV0A0gVnaQ@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, 5 Sept 2026 at 10:45, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Fri, Sep 04, 2026 at 08:55:07PM +1200, David Rowley wrote:
> > 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.
>
> FWIW, I've always been a fan of your compiler magic tricks like this
> one. Even if you did not measure much of a performance difference at
> runtime, less instructions overall across gcc and clang sounds like a
> better deal to me anyway? It sounds like the sort of improvements
> that could be done independently of what is being discussed here.

Thanks. I thought that the only possible reason for not adding the
overflow checks would be for not wanting to add the performance
penalty of the extra jump for something that's quite unlikely. I
thought we could make that argument void if we found a way to remove
one of the existing jumps so that we kept the same number after
including the overflow checks. If the additional overhead of the
overflow checks is not a concern, then I see no reason not to add
them.

Looking at the latest patch, I'd put this comment back to what I wrote:

+ * 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.

The extra sentence has an awful AI whiff to it.

David

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2026-09-05 05:20:45 Re: [PATCH] pg_surgery: check the page header and line pointers
Previous Message Noah Misch 2026-09-05 04:00:20 Re: Add support for COPY TO in tablesync for partitioned tables.