Re: Use pg_neg_s*_overflow() for open-coded negation overflow checks

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Ewan Young <kdbase(dot)hack(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, David Rowley <dgrowleyml(at)gmail(dot)com>, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Subject: Re: Use pg_neg_s*_overflow() for open-coded negation overflow checks
Date: 2026-08-31 07:26:06
Message-ID: apUsjiBcAc0wBIdo@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-08-31 07:29:42 Re: Fix pg_stat_statements losing normalized query text after reset
Previous Message Daniel Gustafsson 2026-08-31 07:16:52 Re: Offline data checksum changes can cause incorrect checksum state on standbys