Re: Add PRODUCT() aggregate function

Hi Jeevan,

Thanks for the explanation and the pro() example, it is convincing.
I hadn't considered this properly earlier. for SUM the overflow
depends only on number of rows, but for PRODUCT it depends on the
values itself, so it will overflow in very few steps whenever values
are more than 1. So native fast path will help only for columns
having mostly 0, 1 or -1, not for large tables in general. I also
checked int128.h and you are right, there is no existing function
for overflow-checked "int128 *= int64" type of multiply, so this
needs new code, not reuse of the SUM(int8) pattern. So agree, fine to
take this up later as a follow-up, no need to block on it.

About your question on float8 vs numeric for float variants - I
would prefer float8. sum(float4)/sum(float8) already return
float4/float8, not numeric, so PRODUCT() staying same for float
types will be more consistent. It also avoids the overflow-primitive
problem for floats, since float just becomes Infinity instead of
erroring out. For int2/int4/int8 numeric is fine as it is.

Thanks,
Vaibhav Dalvi
EnterpriseDB

On Thu, Sep 10, 2026 at 7:26 PM Jeevan Chalke <
jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:

> Thank you, Vaibhav, for the review and the comment. Really appreciate it.
>
> When I started working on this, I did look at Int128AggState and wondered
> whether the same trick could be used for integer products. It can't, at
> least not without new infrastructure.
>
> Int128AggState avoids ever needing an overflow check because the values it
> accumulates stay bounded well within 128 bits for any realistically sized
> table: sum(int8) only accumulates via plain int128 addition, and each
> int64
> input is at most 2^63, so sumX can't overflow until you've summed roughly
> 2^64 rows -- no real table gets remotely close to that.
>
> PRODUCT has the opposite problem: it's the multiply itself that can
> overflow, and there's currently no overflow-checked 128-bit multiply
> primitive in int128.h to build on. Adding one -- plus the serialize/
> deserialize/combine plumbing an internal transition type would need --
> felt like overkill for an initial feature. I'd rather land PRODUCT() as
> proposed and treat this as a follow-up optimization once it's in use.
>
> On Thu, Sep 10, 2026 at 4:39 PM Vaibhav Dalvi <
> vaibhav(dot)dalvi(at)enterprisedb(dot)com> wrote:
>
>> Hi Jeevan,
>>
>> Nice feature; I tested it locally and it works correctly. NULL
>> handling, parallel aggregate (combine), and the moving-window.
>> Fallback to recalculation are all fine, no correctness bug was found.
>> I only have the following point with a short description.
>>
>> *There is no fast path for the common case; it always goes through
>> Numeric:*
>> For int2/int4/int8/float4/float8, every row undergoes a full
>> arbitrary-precision
>> Numeric conversion plus numeric_mul, even when the running product
>> would easily fit in int64/int128 for most rows. This file already has a
>> pattern
>> for exactly this problem (int8 SUM uses int128 internally, only promoting
>> to
>> numeric on real overflow).
>>
>
> I did a quick test to see how fast that "fits in int64" window closes,
> multiplying the same number in a loop:
>
> create or replace function pro(a int, b int) returns bigint as $$
> declare
> p bigint default 1;
> begin
> for i in 1 .. a loop
> p := p * b;
> end loop;
> return p;
> end; $$ language plpgsql;
>
> # select pro(100, 2);
> ERROR: bigint out of range
>
> # select pro(5, 32767);
> ERROR: bigint out of range
>
> Multiplying 2 by itself overflows bigint well before 100 iterations (2^63
> is the limit), and multiplying by the max smallint value overflows in just
> 5. Since PRODUCT() grows multiplicatively, the bigint/int128 range gets
> exhausted very quickly for realistic inputs -- which is why I promoted to
> numeric from the start rather than trying to stay native.
>
>
>> I think PRODUCT(int4)/PRODUCT(int2) over a
>> large table will be much slower per row than SUM for the same data,
>> because
>> of this.
>>
>> So, if possible, consider using the same
>> native-then-promote-on-overflow approach here.
>>
>
> The same reasoning applies to the float variants. That said, I'm open to
> returning float8 for those instead, despite its narrower range than
> numeric, if reviewers prefer that.
>
> Thanks,
>
>
>>
>> Thanks,
>> Vaibhav Dalvi
>> EnterpriseDB
>>
>> On Fri, Jun 26, 2026 at 11:24 AM Jeevan Chalke <
>> jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:
>>
>>> Hello,
>>>
>>> CFbot flagged this for a rebase. The conflicts were due to the catalog
>>> version bump, so I've dropped it here and noted in the commit message
>>> that the committer should bump catversion at commit time to avoid
>>> recurring conflicts.
>>>
>>> Also added tests as suggested by Jim.
>>>
>>> Thanks
>>>
>>> On Tue, Jun 23, 2026 at 5:26 PM Jeevan Chalke <
>>> jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Jun 23, 2026 at 4:32 PM Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>
>>>> wrote:
>>>>
>>>>> Hi Jeevan
>>>>>
>>>>> On 23/06/2026 10:37, Dean Rasheed wrote:
>>>>> > On Tue, 23 Jun 2026 at 08:49, Jeevan Chalke
>>>>> > <jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:
>>>>> >> PRODUCT() returns the product of all non-null input values. It is
>>>>> defined for
>>>>> >> int2, int4, int8, float4, float8 and numeric input, and always
>>>>> returns numeric.
>>>>> > I don't think that you need to define it for all those types. I
>>>>> > suspect that you could just define it for numeric and float8, and let
>>>>> > implicit casting do the rest.
>>>>>
>>>>> +1
>>>>>
>>>>> I've tested the patch in many different scenarios and all results look
>>>>> fine -- valgrind also didn't report anything :)
>>>>>
>>>>> The test coverage is comprehensive! For the sake of completeness I'd
>>>>> add
>>>>> numeric tests for NaN and Infitinty with positive numeric values in the
>>>>> set, e.g:
>>>>>
>>>>> postgres=# WITH j (v) AS (VALUES
>>>>> ('NaN'::numeric),('Infinity'::numeric),(3.14))
>>>>> SELECT product(v) FROM j;
>>>>> product
>>>>> ---------
>>>>> NaN
>>>>> (1 row)
>>>>>
>>>>> Other than that and the point mentioned by Dean I have nothing to add
>>>>> at
>>>>> this point.
>>>>>
>>>>
>>>> Thanks, Jim, for the thorough testing.
>>>>
>>>> I'll include that test case in the next version of the patch.
>>>>
>>>>
>>>>
>>>>>
>>>>> Thanks for the patch.
>>>>>
>>>>> Best, Jim
>>>>>
>>>>
>>>>
>>>> --
>>>> *Jeevan Chalke*
>>>> *Senior Principal Engineer, Engineering Manager*
>>>> *Product Development*
>>>>
>>>> enterprisedb.com <https://www.enterprisedb.com>
>>>>
>>>
>>>
>>> --
>>> *Jeevan Chalke*
>>> *Senior Principal Engineer, Engineering Manager*
>>> *Product Development*
>>>
>>> enterprisedb.com <https://www.enterprisedb.com>
>>>
>>
>
> --
> *Jeevan Chalke*
> *Senior Principal Engineer, Engineering Manager*
> *Product Development*
>
> enterprisedb.com <https://www.enterprisedb.com>
>