Re: right() returns the whole string for the most negative n

On Tue, Aug 25, 2026 at 8:18 PM Dagfinn Ilmari Mannsåker
<ilmari(at)ilmari(dot)org> wrote:
>
> Ewan Young <kdbase(dot)hack(at)gmail(dot)com> 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