Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG

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)