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?
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()?
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. We have a few other uselocale() calls in
pg_locale.c, and we take care to restore the old locale in those.
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.
--
Heikki Linnakangas
Neon (https://neon.tech)
