Hi Jeevan, Patch looks good to me. No more comments.
Regards, Vaibhav On Thu, Sep 24, 2026 at 1:09 PM Jeevan Chalke < [email protected]> wrote: > > > On Tue, Sep 22, 2026 at 7:48 PM Jeevan Chalke < > [email protected]> wrote: > >> Hello, >> >> On Sun, Sep 13, 2026 at 12:23 PM Vaibhav Dalvi < >> [email protected]> wrote: >> >>> 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 (and Dean, Jim, back in June) for pushing on this -- >> agreed, >> we'll switch product(float4)/product(float8) to return float8 natively >> rather >> than numeric, consistent with sum(float4)/sum(float8). >> >> Will post v3 with this change (float4/float8 -> float8) soon. >> > > v3 attached. > > product(float4)/product(float8) now return double precision natively (no > longer numeric), matching sum(float4)/sum(float8). > > Note: Like sum(), a finite product that would overflow double precision > raises an error rather than silently becoming Infinity -- Infinity only > results when it's already present in the input. Added a regression test > covering this. > > Thanks, > > -- > *Jeevan Chalke* > *Senior Principal Engineer, Engineering Manager* > *Product Development* > > enterprisedb.com <https://www.enterprisedb.com> >
