Re: tiny step toward threading: reduce dependence on setlocale()

On 08.08.24 22:00, Jeff Davis wrote:
> On Wed, 2024-08-07 at 22:44 +0200, Peter Eisentraut wrote:
>> But after this patch set, locale cannot be NULL anymore, so the third
>> branch is obsolete.
> 
> ...
> 
>> Second, there are a number of functions in like.c like the above that
>> take separate arguments like pg_locale_t locale, bool locale_is_c.
>> Because pg_locale_t now contains the locale_is_c information, these
>> can
>> be combined.
> 
> I believe these patches are correct, but the reasoning is fairly
> complex:
> 
> 1. Some MatchText variants are called with 0 for locale. But that's OK
> because ...
> 
> 2. A MatchText variant only cares about the locale if MATCH_LOWER(t) is
> defined, and ...
> 
> 3. Only one variant, SB_IMatchText() defines MATCH_LOWER(), and ...
> 
> 4. SB_IMatchText() is called with a non-zero locale.
> 
> All of these are a bit confusing to follow because it's generated code.
> #2 is particularly non-obvious, because "locale" is not even an
> argument of the MATCH_LOWER(t) or GETCHAR(t) macros, it's taken
> implicitly from the outer scope.
> 
> I don't think your patches cause this confusion, but is there a way you
> can clarify some of this along the way?

Yes, this is also my analysis.  The patch in 
<https://www.postgresql.org/message-id/flat/700d2e86-bf75-4607-9cf2-f5b7802f6e88@eisentraut.org> 
would replace passing 0 with an actual locale object.  The changes in 
GenericMatchText() could also be applied independently so that we'd 
always pass in a non-zero locale value, even if it would not be used in 
some cases.  I need to update that patch to cover your latest changes. 
I'll see if I can propose something here that looks a bit nicer.