Re: Add PRODUCT() aggregate function

Поиск
Список
Период
Сортировка
Искать

Re: Add PRODUCT() aggregate function

От:
Jim Jones <jim.jones@uni-muenster.de>
Дата:
Hi Jeevan

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

Best, Jim


Re: Add PRODUCT() aggregate function

От:
Vaibhav Dalvi <vaibhav.dalvi@enterprisedb.com>
Дата:
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.chalke@enterprisedb.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.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

Re: Add PRODUCT() aggregate function

От:
Dean Rasheed <dean.a.rasheed@gmail.com>
Дата:
On Tue, 23 Jun 2026 at 09:37, Dean Rasheed  wrote:
>
> On Tue, 23 Jun 2026 at 08:49, Jeevan Chalke
>  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.

... and perhaps make the float8 version return float8.

Regards,
Dean


Re: Add PRODUCT() aggregate function

От:
Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Дата:
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

Re: Add PRODUCT() aggregate function

От:
Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Дата:


On Tue, Jun 23, 2026 at 2:13 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
On Tue, 23 Jun 2026 at 09:37, Dean Rasheed <dean.a.rasheed@gmail.com> 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.

Thank you, Dean, for looking at this.

I appreciate the suggestion, but I don't think the implicit-casting approach
works well here, for the following reasons:

1. The integer types (int2/int4/int8) have implicit casts to both float8 and
numeric.  Since float8 is the preferred type in the numeric type category,
the function resolution machinery would select the product(float8) variant.
That has two consequences: an extra cast function has to be executed per row,
and, more importantly, integer inputs would be accumulated as float8.
For a large product that yields a lossy result in exponent form, whereas
accumulating in numeric gives an exact answer.

2. This approach is also consistent with the existing aggregates —
sum(), avg(), min()/max(), etc. all define per-type variants rather than
relying on implicit input casting. The patch follows that established
pattern rather than introducing a new one.

3. There is also a small performance penalty to the casting approach:
the per-row execution of the cast function itself, in addition to the
resolution issue noted in (1).

I ran a quick test to demonstrate point (1). For large products, the float8
implementation returns a lossy, exponential result like 9.9999970000003e+20,
whereas numeric returns the exact value of 999999700000029999999.

I would be happy to share the test script if it's helpful.

 

... and perhaps make the float8 version return float8.

Yes, I already noted this under "Open questions" in my first email. We can
certainly make the float4 and float8 variants return float8 instead of
numeric. Let's see what others think before making that adjustment.

Thanks
 

Regards,
Dean


--
Jeevan Chalke
Senior Principal Engineer, Engineering Manager
Product Development


enterprisedb.com

Re: Add PRODUCT() aggregate function

От:
Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Дата:


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
FAQ