On Thu, Aug 27, 2026 at 01:57:59PM +0800, Ewan Young wrote:
> No behavioral change: each site keeps its existing hard- or soft-error
> path.


-       if (value == PG_INT64_MIN)
+       if (pg_neg_s64_overflow(value, &result))

One could ask why you don't add an unlikely() here.  But it looks to
me that this locaton is right and that you could just remove the
unlikely() from all the places where the overflow routines are used
anyway, because they already embed some unlikely() calls for the
minimum bound checks.

+   if (arg1 < 0 && unlikely(pg_neg_s32_overflow(arg1, &result)))
        ereport(ERROR, 

Well, this should embed both conditions but we don't need an unlikely
here anyway.  That could also be written as the following, which seems
slightly better from here in terms of the abs functions:
if (arg1 < 0)
{
    if (neg_overflow())
        ereport(ERROR)
}
else
    result = arg1;

-   /* If the result is INT64_MIN, it cannot be represented. */

Comment removed.  Perhaps it should not.

-       if (unlikely(c == PG_INT64_MIN))
+       Cash        result;

This pattern introduced in cash_div_int64() is inconsistent with the
code in cash_mul_int64() a couple of lines above.
--
Michael

Attachment: signature.asc
Description: PGP signature

Reply via email to