From: | "Tristan Partin" <tristan(at)neon(dot)tech> |
---|---|
To: | <joe(at)cd>, "Heikki Linnakangas" <hlinnaka(at)iki(dot)fi>, "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, <gdo(at)leader(dot)it> |
Cc: | "PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG |
Date: | 2023-07-03 14:42:58 |
Message-ID: | CTSM7EZZMY12.1F7GV7Y2KPBBD@gonk |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs pgsql-hackers |
On Fri Jun 30, 2023 at 7:13 AM CDT, Joe Conway wrote:
> On 6/29/23 22:13, Tristan Partin wrote:
> > On Mon Jun 5, 2023 at 11:00 AM CDT, Heikki Linnakangas wrote:
> >> I think the uselocale() call renders ineffective the setlocale() calls
> >> that we make later. Maybe we should replace our setlocale() calls with
> >> uselocale(), too.
> >
> > For what it's worth to everyone else in the thread (especially Joe), I
> > have a patch locally that fixes the mentioned bug using uselocale(). I
> > am not sure that it is worth committing for v16 given how _large_ (the
> > patch is actually quite small, +216 -235) of a change it is. I am going
> > to spend tomorrow combing over it a bit more and evaluating other
> > setlocale uses in the codebase.
>
> (moving thread to hackers)
>
> I don't see a patch attached -- how is it different than what I posted a
> week ago and added to the commitfest here?
>
> https://commitfest.postgresql.org/43/4413/
>
> FWIW, if you are proposing replacing all uses of setlocale() with
> uselocale() as Heikki suggested:
>
> 1/ I don't think that is pg16 material, and almost certainly not
> back-patchable to earlier.
I am in agreement.
> 2/ It probably does not solve all of the identified issues caused by the
> newer perl libraries by itself, i.e. I believe the patch posted to the
> CF is still needed.
Perhaps. I do think your patch is still valuable regardless. Works for
backpatching and is just good defensive programming. I have added myself
as a reviewer.
> 3/ I believe it is probably the right way to go for pg17+, but I would
> love to hear opinions from Jeff Davis, Peter Eisentraut, and/or Thomas
> Munroe (the locale code "usual suspects" ;-)), and others, about that.
Thanks for your patience. Attached is a patch that should cover all the
problematic use cases of setlocale(). There are some setlocale() calls in
tests, initdb, and ecpg left. I plan to get to ecpglib before the final
version of this patch after I abstract over Windows not having
uselocale(). I think leaving initdb and tests as is would be fine, but I
am also happy to just permanently purge setlocale() from the codebase
if people see value in that. We could also poison[0] setlocale() at that
point.
[0]: https://gcc.gnu.org/onlinedocs/cpp/Pragmas.html
--
Tristan Partin
Neon (https://neon.tech)
Attachment | Content-Type | Size |
---|---|---|
v1-0001-Skip-checking-for-uselocale-on-Windows.patch | text/x-patch | 677 bytes |
v1-0002-Add-locale_is_c-function.patch | text/x-patch | 4.5 KB |
v1-0003-Use-uselocale-instead-of-setlocale.patch | text/x-patch | 26.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Pantelis Theodosiou | 2023-07-03 14:57:52 | Re: group by can use alias from select list but not the having clause |
Previous Message | Tom Lane | 2023-07-03 14:12:11 | Re: group by can use alias from select list but not the having clause |
From | Date | Subject | |
---|---|---|---|
Next Message | Tristan Partin | 2023-07-03 14:49:21 | Re: Make uselocale protection more consistent |
Previous Message | Peter Eisentraut | 2023-07-03 14:26:24 | Re: Optionally using a better backtrace library? |