Re: Add PRODUCT() aggregate function

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>