| From: | Ewan Young <kdbase(dot)hack(at)gmail(dot)com> |
|---|---|
| To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | David Rowley <dgrowleyml(at)gmail(dot)com>, Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Subject: | right() returns the whole string for the most negative n |
| Date: | 2026-08-25 06:57:18 |
| Message-ID: | CAON2xHNnBz-AcPJgDmd5_39+8qR5AUKEZk4X3ZM-0zdsATn8kQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Hackers,
right(text, int) gets one input wrong:
SELECT right('abcdef', (-2147483648)::int4); -- 'abcdef'
SELECT right('abcdef', -2147483647); -- '' (correct)
SELECT right('abcdef', -3); -- 'def' (correct)
A negative n means "return all but the first |n| characters", so |n| that
big has to give an empty string. Only this one value is affected, and
left() is fine.
text_right() negates n to get the number of characters to skip:
if (n < 0)
n = -n;
else
n = pg_mbstrlen_with_len(p, len) - n;
off = pg_mbcharcliplen(p, len, n);
Negating PG_INT32_MIN overflows; under -fwrapv it comes back as
PG_INT32_MIN, still negative, and pg_mbcharcliplen() returns 0 for any
negative limit, so off is 0 and the whole string is returned.
The attached patch clamps to PG_INT32_MAX rather than negating. Any n
whose absolute value reaches the string's length skips all of it, and a
text value can't be longer than PG_INT32_MAX, so the answer is unchanged
for every other input. I did not make it an error, unlike
text_format_string_conversion() a few hundred lines down, which rejects a
width of INT_MIN (73e7025bd8e, complete with a "-INT_MIN is undefined"
comment): a format width has no sensible clamp, whereas an oversized skip
count does.
left() is not affected because its negative case adds n to the character
length instead of negating it, and since the length is non-negative and
bounded by the varlena size limit, that sum cannot overflow.
This dates to 49b27ab5514, which added left()/right() in 2010, and the
line has not been touched since; I could not find a previous report. It's
the same shape as b4dfae2ffac (money, INT64_MIN / -1) from a few weeks
ago.
The patch adds the case to the existing left()/right() test in text.sql,
which currently covers only generate_series(-5, 5). make check passes;
before adding the expected output I ran the suite deliberately and the
only difference was the new line.
--
Regards,
Ewan Young
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Fix-right-with-the-most-negative-integer.patch | application/octet-stream | 3.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tatsuo Ishii | 2026-08-25 07:01:50 | Re: Row pattern recognition |
| Previous Message | Peter Eisentraut | 2026-08-25 06:52:27 | Re: Fix signed/unsigned integer handling in pg_restore_relation_stats() |