| From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
|---|---|
| To: | Ewan Young <kdbase(dot)hack(at)gmail(dot)com> |
| Cc: | David Rowley <dgrowleyml(at)gmail(dot)com>, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrey Rachitskiy <pl0h0yp1(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Subject: | Re: right() returns the whole string for the most negative n |
| Date: | 2026-08-27 11:13:21 |
| Message-ID: | FAAEE5A6-88B2-4638-A07D-47C5827AB638@yesql.se |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On 27 Aug 2026, at 07:17, Ewan Young <kdbase(dot)hack(at)gmail(dot)com> wrote:
> On Thu, Aug 27, 2026 at 10:45 AM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>> Also, pg_neg_s32_overflow() was introduced in 2024 by 0a27c3d0f733,
>> which is v18+. Depending on Daniel's patience for backpatching two
>> different versions, it might be worth just using the == PG_INT32_MIN
>> method to fix the bug, then allowing the follow-up patch to switch
>> that in v20 only. That would mean v18 and v19 don't get the most
>> optimal fix, but it's hard to imagine that's going to be performance
>> critical.
>
> Thanks, that plan makes sense to me -- let's keep the bug fix uniform
> across all branches and do the pg_neg_s32_overflow() switch separately.
FWIW, I agree with this plan.
> Attached v4 goes back to the explicit PG_INT32_MIN test:
>
> if (unlikely(n == PG_INT32_MIN))
> n = PG_INT32_MAX;
> else
> n = -n;
>
> which applies cleanly to every supported branch (I checked REL_14 through
> REL_19 and master). The fix and the regression case are otherwise
> unchanged.
I will prepare a backpatch all the way of this, and will ping the new thread
when done. (There is a lof work being done for shipping v19 so have a little
patience.)
> I'll take the pg_neg_s32_overflow() cleanup to its own thread, master
> only, covering the other open-coded negation checks Ilmari pointed at
> (int2/4/8 um/abs/div, lcm, cash, numericvar_to_int64); once this fix
> lands I'll fold text_right() into that too. So the back-branches keep the
> plain form and only master gets the helper.
+1
--
Daniel Gustafsson
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Borodin | 2026-08-27 11:17:31 | Re: [PATCH] Fix timeline history after recovery stops on an ancestor |
| Previous Message | Osama Abdul Qader | 2026-08-27 11:12:16 | Re: Persist slot invalidations before publishing them |