On Tue, Aug 25, 2026 at 8:18 PM Dagfinn Ilmari Mannsåker <[email protected]> wrote: > > Ewan Young <[email protected]> writes: > > > diff --git a/src/backend/utils/adt/varlena.c > > b/src/backend/utils/adt/varlena.c > > index a09a9e5d5bb..3117069cf1a 100644 > > --- a/src/backend/utils/adt/varlena.c > > +++ b/src/backend/utils/adt/varlena.c > > @@ -4714,7 +4714,17 @@ text_right(PG_FUNCTION_ARGS) > > int off; > > > > if (n < 0) > > - n = -n; > > + { > > + /* > > + * Negating PG_INT32_MIN would overflow, so clamp instead. > > Any n whose > > + * absolute value is at least the string's length skips the > > whole > > + * string, and len can't exceed PG_INT32_MAX, so this is > > equivalent. > > + */ > > + if (unlikely(n == PG_INT32_MIN)) > > + n = PG_INT32_MAX; > > + else > > + n = -n; > > + } > > Instead of open-coding this, how about about using pg_neg_s32_overflow? > > if (pg_neg_s32_overflow(n, &n)) > n = PG_INT32_MAX; >
Much nicer, thanks - done in v2. varlena.c already includes common/int.h,
so no new header was needed.
> This made me think we might want saturating versions of the
> pg_*_overflow functions, but some quick grepping doesn't reveal any
> other places using pg_*_overflow do it manually, so that feels like
> premature generalisation.
Agreed, I left it as the two-liner.
Behaviour and tests are unchanged from v1: right('abcdef', INT32_MIN) now
returns '', the adjacent values and left() are untouched, and make check
passes.
>
> - ilmari
--
Regards,
Ewan Young
v2-0001-Fix-right-with-the-most-negative-integer.patch
Description: Binary data
