Re: Add PRODUCT() aggregate function - Mailing list pgsql-hackers

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.dalvi@enterprisedb.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.chalke@enterprisedb.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.chalke@enterprisedb.com> wrote:

On Tue, Jun 23, 2026 at 4:32 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

Hi Jeevan

On 23/06/2026 10:37, Dean Rasheed wrote:
> On Tue, 23 Jun 2026 at 08:49, Jeevan Chalke
> <jeevan.chalke@enterprisedb.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

--

Jeevan Chalke
Senior Principal Engineer, Engineering Manager
Product Development

enterprisedb.com

--

Jeevan Chalke
Senior Principal Engineer, Engineering Manager
Product Development

enterprisedb.com