Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG - Mailing list pgsql-hackers
From | Joe Conway |
---|---|
Subject | Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG |
Date | |
Msg-id | 41a4898e-1cdb-960d-f17e-d71886407e04@joeconway.com Whole thread Raw |
Responses |
Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG
|
List | pgsql-hackers |
(moving to hackers) On 6/12/23 05:13, Heikki Linnakangas wrote: > On 10/06/2023 22:28, Joe Conway wrote: >> On 6/10/23 15:07, Joe Conway wrote: >>> On 6/10/23 14:42, Tom Lane wrote: >>>> Joe Conway <mail@joeconway.com> writes: >>>>> 5/ The attached fixes the issue for me on pg10 and passes check-world. >>>>> Comments? >>>> >>>> The call in PGLC_localeconv seems *very* oddly placed. Why not >>>> do that before it does any other locale calls? Otherwise you don't >>>> really have reason to believe you're saving the appropriate >>>> values to restore later. >>> >>> >>> As far as I can tell it really only affects localeconv(), so I tried to >>> place it close to those. But I am fine with moving it up. >> >> This version is against pg16 (rather than pg10), moves up that hunk, >> mentions localeconv() in the comment as the reason for the call, and >> fixes some whitespace sloppiness. I will plan to apply to all supported >> branches. >> >> Better? > > The man page for uselocale(LC_GLOBAL_LOCALE) says: "The calling thread's > current locale is set to the global locale determined by setlocale(3)." > Does that undo the effect of calling uselocale() previously, so if you > later call setlocale(), the new locale takes effect in the thread too? > Or is it equivalent to "uselocale(LC_ALL, setlocale(NULL))", so that it > sets the thread's locale to the current global locale, but later > setlocale() calls have no effect on it? setlocale() changes the global locale, but uselocale() changes the locale that is currently active, as I understand it. Also note that uselocale man page says "Unlike setlocale(3), uselocale() does not allow selective replacement of individual locale categories. To employ a locale that differs in only a few categories from the current locale, use calls to duplocale(3) and newlocale(3) to obtain a locale object equivalent to the current locale and modify the desired categories in that object." > In any case, this still doesn't feel like the right place. We have many > more setlocale() calls. Shouldn't we sprinkle them all with > uselocale(LC_GLOBAL_LOCALE)? cache_locale_time() for example. Or rather, > all the places where we use any functions that depend on the current locale. > > How about we replace all setlocale() calls with uselocale()? I don't see us backpatching something that invasive. It might be the right thing to do for pg17, or even pg16, but I think that is a different discussion > Shouldn't we restore the old thread-specific locale after the calls? I'm > not sure why libperl calls uselocale(), but we are now overwriting the > locale that it sets. That is a good question. Though arguably perl is doing the wrong thing by not resetting the global locale when it is being used embedded. > We have a few other uselocale() calls in pg_locale.c, and we take > care to restore the old locale in those. I think as long as we are relying on setlocale rather than uselocale in general (see above), the global locale is where we want things left. > There are a few uselocale() calls in ecpg, and they are protected by > HAVE_USELOCALE. Interestingly, the calls in pg_locale.c are not, but > they are protected by HAVE_LOCALE_T. Seems a little inconsistent. Possibly something we should clean up, but I think that is separate from this fix. In general I think we have 2 or possibly three distinct things here: 1/ how do we fix the misbehavior reported due to libperl in existing stable branches 2/ what makes most sense going forward (and does that mean pg16 or pg17) 3/ misc code cleanups I was mostly trying to concentrate on #1, but 2 & 3 are worthy of discussion. -- Joe Conway PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
pgsql-hackers by date: