Horton

Thread

  1. [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Bryan Green <dbryan.green@gmail.com> — 2025-12-10T00:45:52Z

    Hello hackers,

I've been investigating a performance issue on Windows with recent
gettext versions (0.20.1 and later) that causes exception-heavy
workloads to run significantly slower than with gettext 0.19.8.

Starting with gettext 0.20.1, the library changed its Windows locale
handling in a way that conflicts with how PostgreSQL sets LC_MESSAGES.
The performance regression manifests when raising many exceptions:

  - gettext 0.19.8: ~32 seconds for 1M exceptions
  - gettext 0.20.1+: ~180 seconds for 1M exceptions
  - gettext 0.2x.y+: ~39 seconds for 1M exceptions

The root cause is a combination of three issues:

1. Locale format mismatch
   gettext 0.20.1+ introduced a get_lcid() function that expects Windows
locale format ("English_United States.1252") rather than POSIX format
("en_US"). This function enumerates all Windows locales (~259) until a
match is found, then uses the resulting LCID to determine the catalog path.

   PostgreSQL, however, has always used IsoLocaleName() to convert
Windows locales to POSIX format before setting LC_MESSAGES. This means
we're passing "en_US" to a function expecting "English_United States.1252".

   The enumeration doesn't find "en_US" among Windows locale names,
returns 0, and gettext falls back to its internal locale resolution
(which still works correctly - translations are not broken, just slow).

2. Missing cache on failure
   The get_lcid() function has a cache, but it only updates the cache
when found_lcid > 0 (successful lookup). Failed lookups don't update the
cache, causing the 259-locale enumeration to repeat on every gettext() call.

   This is the actual performance bug in gettext - even if we passed a
valid Windows locale format, setting lc_messages to 'C' or 'POSIX'
(common in scripts and automation) would trigger the same issue since
these aren't Windows locale names.  Please see the bug I opened with the
gettext project [1].

3. Empty string bug in early 0.2x.y
   gettext 0.20.1 introduced a setlocale_null() wrapper that returns ""
instead of NULL when setlocale() fails. This causes get_lcid("") to be
called, triggering the enumeration bug even when LC_MESSAGES is unset.


The attached patch takes a pragmatic approach: for gettext 0.20.1+, we
avoid triggering the bug by using Windows locale format instead of
calling IsoLocaleName(). This works because gettext 0.20.1+ internally
converts the Windows format back to POSIX for catalog lookups, whereas
0.19.8 and earlier need POSIX format directly.

The patch uses LIBINTL_VERSION to detect the gettext version at compile
time and adjusts behavior accordingly. When locale is NULL, empty, or
set to 'C'/'POSIX', we fall back to using the LC_CTYPE value (which is
already in Windows format and always set).

For gettext 0.19.8 and earlier, the existing IsoLocaleName() path is
retained to maintain compatibility.

I don't have automated tests for this since we'd need to test against
multiple versions of a third-party library. I'm open to suggestions if
folks think we should add something to the buildfarm or CI.

Manual testing can be done with this test case:

-- Create test table
CREATE TABLE sampletest (
    a VARCHAR,
    b VARCHAR
);

-- Insert 1 million rows with random data
INSERT INTO sampletest (a, b)
SELECT
    substr(md5(random()::text), 0, 15),
    (100000000 * random())::integer::varchar
FROM generate_series(1, 1000000);

-- Create function that converts string to float with exception handling
CREATE OR REPLACE FUNCTION toFloat(str VARCHAR, val REAL)
RETURNS REAL AS $$
BEGIN
    RETURN CASE
        WHEN str IS NULL THEN val
        ELSE str::REAL
    END;
EXCEPTION
    WHEN OTHERS THEN
        RETURN val;
END;
$$ LANGUAGE plpgsql
   COST 1
   IMMUTABLE;

-- Test query to trigger 1M exceptions
-- (all conversions will fail since we inserted random MD5 strings)
\timing on
SELECT MAX(toFloat(a, NULL)) FROM sampletest;

The ~8 second difference is due to the initial enumeration and other
coding changes that were made by gettext.  Keep in mind that for 1M
exceptions we are probably calling gettext 2-3 million times.

-- 
Bryan Green
EDB: https://www.enterprisedb.com

[1] https://savannah.gnu.org/bugs/?67781

  2. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Peter Eisentraut <peter@eisentraut.org> — 2025-12-11T14:43:36Z

    On 10.12.25 01:45, Bryan Green wrote:
> The attached patch takes a pragmatic approach: for gettext 0.20.1+, we
> avoid triggering the bug by using Windows locale format instead of
> calling IsoLocaleName(). This works because gettext 0.20.1+ internally
> converts the Windows format back to POSIX for catalog lookups, whereas
> 0.19.8 and earlier need POSIX format directly.

I can confirm that this patch fixes the performance deviation from 
activating --enable-nls on Windows (tested with MSYS2/UCRT64).

I wonder, this change that gettext did with the locale naming, does that 
also affect what guidance we need to provide to users about how to 
configure locale names?  For example, on a Unix-ish system, a user can 
do something like initdb ... --lc-messages=de_DE.  What locale name 
format do you need to use on Windows to get the translations to 
activate?  Does this also depend on the gettext version?

  3. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Andres Freund <andres@anarazel.de> — 2025-12-11T16:05:12Z

    Hi,

On 2025-12-11 15:43:36 +0100, Peter Eisentraut wrote:
> On 10.12.25 01:45, Bryan Green wrote:
> > The attached patch takes a pragmatic approach: for gettext 0.20.1+, we
> > avoid triggering the bug by using Windows locale format instead of
> > calling IsoLocaleName(). This works because gettext 0.20.1+ internally
> > converts the Windows format back to POSIX for catalog lookups, whereas
> > 0.19.8 and earlier need POSIX format directly.
> 
> I can confirm that this patch fixes the performance deviation from
> activating --enable-nls on Windows (tested with MSYS2/UCRT64).

FWIW, Bilal and I had, IIRC, explicitly not enabled on windows CI because it
made the build process even slower. But perhaps we should re-measure the
difference and re-consider?

Greetings,

Andres Freund

  4. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Bryan Green <dbryan.green@gmail.com> — 2025-12-11T17:22:11Z

    On 12/11/2025 8:43 AM, Peter Eisentraut wrote:
> On 10.12.25 01:45, Bryan Green wrote:
>> The attached patch takes a pragmatic approach: for gettext 0.20.1+, we
>> avoid triggering the bug by using Windows locale format instead of
>> calling IsoLocaleName(). This works because gettext 0.20.1+ internally
>> converts the Windows format back to POSIX for catalog lookups, whereas
>> 0.19.8 and earlier need POSIX format directly.
> 
> I can confirm that this patch fixes the performance deviation from
> activating --enable-nls on Windows (tested with MSYS2/UCRT64).
> 
> I wonder, this change that gettext did with the locale naming, does that
> also affect what guidance we need to provide to users about how to
> configure locale names?  For example, on a Unix-ish system, a user can
> do something like initdb ... --lc-messages=de_DE.  What locale name
> format do you need to use on Windows to get the translations to
> activate?  Does this also depend on the gettext version?
> 
If the language catalogue is installed then they will get translated
messages as expected.  The downside is that because they are passing a
posix locale name then gettext will still do the enumeration everytime.
This will have the negative performance impact.  The good news is that
gettext has accepted my cache patch for their next release.  If a
Windows system is configured with lc_messages="de_DE", but has the next
release of gettext-- they should be fine.  If they don't have the next
release of gettext-- they will notice the performance issue, but that
can be fixed by just changing to from "de_DE" to the correct Windows
locale name.


Walk-through:


1. LCID Lookup: get_lcid("de_DE")
   - Enumerates Windows locales looking for "de_DE"
   - Fails: Windows locales are named "German_Germany", not "de_DE"
   - Returns: 0
   - BUG: Doesn't cache the failure, repeats on every call (patched on
next gettext release)

2. Catalog Search: _nl_make_l10nflist()
   - Tries: locale/de_DE/LC_MESSAGES/postgres-19.mo (not found)
   - Tries: locale/de/LC_MESSAGES/postgres-19.mo (found!)
   - Loads German translations
   - Success!

So, the user gets German messages (catalog fallback works) but
performance is poor (LCID lookup repeats every time) because we don't
cache the failed locale search.



More detailed information for the curious:

Even though get_lcid() returned 0, gettext continues with catalog lookup:

  Function: _nl_find_domain() and _nl_make_l10nflist()
  Location: gettext-runtime/intl/dcigettext.c and l10nflist.c

  Process:
    1. Parse "de_DE" into components:
       language = "de"
       territory = "DE"
       codeset = NULL
       modifier = NULL

    2. Try catalog paths in order (most specific to least specific):

       Try #1: language + territory + codeset + modifier
         Path: /share/locale/de_DE.UTF-8@euro/LC_MESSAGES/postgres-19.mo
         stat(): File not found

       Try #2: language + territory + codeset
         Path: /share/locale/de_DE.UTF-8/LC_MESSAGES/postgres-19.mo
         stat(): File not found

       Try #3: language + territory
         Path: /share/locale/de_DE/LC_MESSAGES/postgres-19.mo
         stat(): File not found (PostgreSQL doesn't ship de_DE)

       Try #4: language + codeset
         Path: /share/locale/de.UTF-8/LC_MESSAGES/postgres-19.mo
         stat(): File not found

       Try #5: language only
         Path: /share/locale/de/LC_MESSAGES/postgres-19.mo
         stat(): SUCCESS! File exists ✓

    3. Load catalog: _nl_load_domain()
       Parse .mo file, load German translations

    4. Look up message: _nl_find_msg()
       Binary search for "division by zero"
       Find translation: "Teilung durch Null"

    5. Return translated message


You might be wondering what happens if the "de" catalog doesn't exist?
It depends on whether the user has set the environment variable LANGUAGE
for their preferred ordered list of languages.  On Windows you can also
set this in the registry. Gettext figures this out.  If LANGUAGE is not
set on Windows then Gettext uses GetUserDefaultUILanguage() to determine
what locale to use. If everything fails, you would get back the msgid
you sent in to start with...so, untranslated.

-- 
Bryan Green
EDB: https://www.enterprisedb.com

  5. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Bryan Green <dbryan.green@gmail.com> — 2025-12-11T17:45:01Z

    On 12/11/2025 10:05 AM, Andres Freund wrote:
> Hi,
> 
> On 2025-12-11 15:43:36 +0100, Peter Eisentraut wrote:
>> On 10.12.25 01:45, Bryan Green wrote:
>>> The attached patch takes a pragmatic approach: for gettext 0.20.1+, we
>>> avoid triggering the bug by using Windows locale format instead of
>>> calling IsoLocaleName(). This works because gettext 0.20.1+ internally
>>> converts the Windows format back to POSIX for catalog lookups, whereas
>>> 0.19.8 and earlier need POSIX format directly.
>>
>> I can confirm that this patch fixes the performance deviation from
>> activating --enable-nls on Windows (tested with MSYS2/UCRT64).
> 
> FWIW, Bilal and I had, IIRC, explicitly not enabled on windows CI because it
> made the build process even slower. But perhaps we should re-measure the
> difference and re-consider?
> 
> Greetings,
> 
> Andres Freund
As long as you use Windows locale names once this patch is in place.
Posix locale names will still incur the performance hit until the next
gettext release. Once using the next gettext release there will not be a
performance penalty for using an invalid locale on Windows.


-- 
Bryan Green
EDB: https://www.enterprisedb.com

  6. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Andres Freund <andres@anarazel.de> — 2025-12-11T17:49:21Z

    Hi,

On 2025-12-11 11:45:01 -0600, Bryan Green wrote:
> On 12/11/2025 10:05 AM, Andres Freund wrote:
> > On 2025-12-11 15:43:36 +0100, Peter Eisentraut wrote:
> >> On 10.12.25 01:45, Bryan Green wrote:
> >>> The attached patch takes a pragmatic approach: for gettext 0.20.1+, we
> >>> avoid triggering the bug by using Windows locale format instead of
> >>> calling IsoLocaleName(). This works because gettext 0.20.1+ internally
> >>> converts the Windows format back to POSIX for catalog lookups, whereas
> >>> 0.19.8 and earlier need POSIX format directly.
> >>
> >> I can confirm that this patch fixes the performance deviation from
> >> activating --enable-nls on Windows (tested with MSYS2/UCRT64).
> > 
> > FWIW, Bilal and I had, IIRC, explicitly not enabled on windows CI because it
> > made the build process even slower. But perhaps we should re-measure the
> > difference and re-consider?
> > 
> > Greetings,
> > 
> > Andres Freund
> As long as you use Windows locale names once this patch is in place.
> Posix locale names will still incur the performance hit until the next
> gettext release. Once using the next gettext release there will not be a
> performance penalty for using an invalid locale on Windows.

What I was referring to was that *building* with NLS support is slower than
building without, which is the reason why CI currently isn't testing NLS in
the "Windows - Server 2022, MinGW64 - Meson" task. Even with ccache, the CI
builds with mingw are pretty darn slow, adding the overhead of creating a good
number of additional files is (or was, haven't retested this recently) making
it even slower.

Greetings,

Andres Freund

  7. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Nazir Bilal Yavuz <byavuz81@gmail.com> — 2025-12-12T09:18:09Z

    Hi,

On Thu, 11 Dec 2025 at 20:49, Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2025-12-11 11:45:01 -0600, Bryan Green wrote:
> > On 12/11/2025 10:05 AM, Andres Freund wrote:
> > > On 2025-12-11 15:43:36 +0100, Peter Eisentraut wrote:
> > >> On 10.12.25 01:45, Bryan Green wrote:
> > >>> The attached patch takes a pragmatic approach: for gettext 0.20.1+, we
> > >>> avoid triggering the bug by using Windows locale format instead of
> > >>> calling IsoLocaleName(). This works because gettext 0.20.1+ internally
> > >>> converts the Windows format back to POSIX for catalog lookups, whereas
> > >>> 0.19.8 and earlier need POSIX format directly.
> > >>
> > >> I can confirm that this patch fixes the performance deviation from
> > >> activating --enable-nls on Windows (tested with MSYS2/UCRT64).
> > >
> > > FWIW, Bilal and I had, IIRC, explicitly not enabled on windows CI because it
> > > made the build process even slower. But perhaps we should re-measure the
> > > difference and re-consider?
> > >
> > > Greetings,
> > >
> > > Andres Freund
> > As long as you use Windows locale names once this patch is in place.
> > Posix locale names will still incur the performance hit until the next
> > gettext release. Once using the next gettext release there will not be a
> > performance penalty for using an invalid locale on Windows.
>
> What I was referring to was that *building* with NLS support is slower than
> building without, which is the reason why CI currently isn't testing NLS in
> the "Windows - Server 2022, MinGW64 - Meson" task. Even with ccache, the CI
> builds with mingw are pretty darn slow, adding the overhead of creating a good
> number of additional files is (or was, haven't retested this recently) making
> it even slower.

I tested this and the timings (minute:seconds) of running tests:

MinGW + NLS [1]: 16:01
MinGW + Patch + NLS [2]: 13:57

I ran the CI again to make sure and the speed up was similar.

[1] https://cirrus-ci.com/task/5944143274311680
[2] https://cirrus-ci.com/task/5477244862201856

-- 
Regards,
Nazir Bilal Yavuz
Microsoft

  8. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-12T20:01:12Z

    Bryan Green <dbryan.green@gmail.com> writes:
> On 12/11/2025 8:43 AM, Peter Eisentraut wrote:
>> I wonder, this change that gettext did with the locale naming, does that
>> also affect what guidance we need to provide to users about how to
>> configure locale names?  For example, on a Unix-ish system, a user can
>> do something like initdb ... --lc-messages=de_DE.  What locale name
>> format do you need to use on Windows to get the translations to
>> activate?  Does this also depend on the gettext version?

> If the language catalogue is installed then they will get translated
> messages as expected.  The downside is that because they are passing a
> posix locale name then gettext will still do the enumeration everytime.
> This will have the negative performance impact.  The good news is that
> gettext has accepted my cache patch for their next release.  If a
> Windows system is configured with lc_messages="de_DE", but has the next
> release of gettext-- they should be fine.  If they don't have the next
> release of gettext-- they will notice the performance issue, but that
> can be fixed by just changing to from "de_DE" to the correct Windows
> locale name.

So IIUC, POSIX-style lc_messages settings do still work on Windows and
will continue to do so, they just incur some extra overhead with
current gettext versions?

If that's the case, I'm inclined to leave my NLS-testing patch [1] as-is,
unconditionally setting a POSIX lc_messages value.  I had anticipated
adding some logic to it to select a Windows locale name when on
Windows, but that seems rather messy, and it's not even clear that the
test would net out faster.  It only needs to do a dozen or so message
lookups, which has to be set against the time needed to identify what
platform we are running on.

			regards, tom lane

[1] https://www.postgresql.org/message-id/1038674.1765568967%40sss.pgh.pa.us

  9. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Bryan Green <dbryan.green@gmail.com> — 2025-12-12T20:56:18Z

    On 12/12/2025 2:01 PM, Tom Lane wrote:
> Bryan Green <dbryan.green@gmail.com> writes:
>> On 12/11/2025 8:43 AM, Peter Eisentraut wrote:
>>> I wonder, this change that gettext did with the locale naming, does that
>>> also affect what guidance we need to provide to users about how to
>>> configure locale names?  For example, on a Unix-ish system, a user can
>>> do something like initdb ... --lc-messages=de_DE.  What locale name
>>> format do you need to use on Windows to get the translations to
>>> activate?  Does this also depend on the gettext version?
> 
>> If the language catalogue is installed then they will get translated
>> messages as expected.  The downside is that because they are passing a
>> posix locale name then gettext will still do the enumeration everytime.
>> This will have the negative performance impact.  The good news is that
>> gettext has accepted my cache patch for their next release.  If a
>> Windows system is configured with lc_messages="de_DE", but has the next
>> release of gettext-- they should be fine.  If they don't have the next
>> release of gettext-- they will notice the performance issue, but that
>> can be fixed by just changing to from "de_DE" to the correct Windows
>> locale name.
> 
> So IIUC, POSIX-style lc_messages settings do still work on Windows and
> will continue to do so, they just incur some extra overhead with
> current gettext versions?
> 
> If that's the case, I'm inclined to leave my NLS-testing patch [1] as-is,
> unconditionally setting a POSIX lc_messages value.  I had anticipated
> adding some logic to it to select a Windows locale name when on
> Windows, but that seems rather messy, and it's not even clear that the
> test would net out faster.  It only needs to do a dozen or so message
> lookups, which has to be set against the time needed to identify what
> platform we are running on.
> 
> 			regards, tom lane
> 
> [1] https://www.postgresql.org/message-id/1038674.1765568967%40sss.pgh.pa.us
Correct.  The translation done for 1M exceptions is what was used as a
benchmark.  That turns into a few million calls to gettext.  Your case
seems like it would not really notice the difference.

-- 
Bryan Green
EDB: https://www.enterprisedb.com

  10. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Thomas Munro <thomas.munro@gmail.com> — 2025-12-15T18:23:41Z

    On Wed, Dec 10, 2025 at 1:45 PM Bryan Green <dbryan.green@gmail.com> wrote:
> 1. Locale format mismatch
>    gettext 0.20.1+ introduced a get_lcid() function that expects Windows
> locale format ("English_United States.1252") rather than POSIX format
> ("en_US"). This function enumerates all Windows locales (~259) until a
> match is found, then uses the resulting LCID to determine the catalog path.

I'm surprised there isn't a fast path that skips all that deprecated
(?) stuff when you use BCP 47 language codes...

  11. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Peter Eisentraut <peter@eisentraut.org> — 2025-12-15T19:26:11Z

    On 12.12.25 10:18, Nazir Bilal Yavuz wrote:
>>>> FWIW, Bilal and I had, IIRC, explicitly not enabled on windows CI because it
>>>> made the build process even slower. But perhaps we should re-measure the
>>>> difference and re-consider?
>>>>
>>>> Greetings,
>>>>
>>>> Andres Freund
>>> As long as you use Windows locale names once this patch is in place.
>>> Posix locale names will still incur the performance hit until the next
>>> gettext release. Once using the next gettext release there will not be a
>>> performance penalty for using an invalid locale on Windows.
>>
>> What I was referring to was that *building* with NLS support is slower than
>> building without, which is the reason why CI currently isn't testing NLS in
>> the "Windows - Server 2022, MinGW64 - Meson" task. Even with ccache, the CI
>> builds with mingw are pretty darn slow, adding the overhead of creating a good
>> number of additional files is (or was, haven't retested this recently) making
>> it even slower.
> 
> I tested this and the timings (minute:seconds) of running tests:
> 
> MinGW + NLS [1]: 16:01
> MinGW + Patch + NLS [2]: 13:57
> 
> I ran the CI again to make sure and the speed up was similar.

Andres was asking about the build time with and without NLS.

There is a note in .cirrus.tasks.yml:

     # Keep -Dnls explicitly disabled, as the number of files it creates 
causes a
     # noticeable slowdown.


I have been testing this a bit.  Locally, using MinGW, I was not able to 
detect any significant difference.  On CI runs, the numbers were to 
erratic to get any consistent sense.  The CI image for Visual Studio 
does not appear to have gettext tools installed, so I couldn't activate 
it there, and I don't have Visual Studio set up locally, so I haven't 
been able to test that.

  12. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Nazir Bilal Yavuz <byavuz81@gmail.com> — 2025-12-15T21:01:19Z

    Hi,

On Mon, 15 Dec 2025 at 22:26, Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 12.12.25 10:18, Nazir Bilal Yavuz wrote:
> >>>> FWIW, Bilal and I had, IIRC, explicitly not enabled on windows CI because it
> >>>> made the build process even slower. But perhaps we should re-measure the
> >>>> difference and re-consider?
> >>>>
> >>>> Greetings,
> >>>>
> >>>> Andres Freund
> >>> As long as you use Windows locale names once this patch is in place.
> >>> Posix locale names will still incur the performance hit until the next
> >>> gettext release. Once using the next gettext release there will not be a
> >>> performance penalty for using an invalid locale on Windows.
> >>
> >> What I was referring to was that *building* with NLS support is slower than
> >> building without, which is the reason why CI currently isn't testing NLS in
> >> the "Windows - Server 2022, MinGW64 - Meson" task. Even with ccache, the CI
> >> builds with mingw are pretty darn slow, adding the overhead of creating a good
> >> number of additional files is (or was, haven't retested this recently) making
> >> it even slower.
> >
> > I tested this and the timings (minute:seconds) of running tests:
> >
> > MinGW + NLS [1]: 16:01
> > MinGW + Patch + NLS [2]: 13:57
> >
> > I ran the CI again to make sure and the speed up was similar.
>
> Andres was asking about the build time with and without NLS.

You are right. I run CI again and compared build times now:

MinGW - NLS [1]: 05:59
MinGW + NLS [2]: 05:55
MinGW + Patch - NLS [3]: 06:42
MinGW + Patch + NLS [4]: 06:04

> I have been testing this a bit.  Locally, using MinGW, I was not able to
> detect any significant difference.

I did not detect any on the CI MinGW task either.

> On CI runs, the numbers were to
> erratic to get any consistent sense.

Yes, I realized that. If you have not cleared the task cache, clearing
it helps to some degree.

[1] https://cirrus-ci.com/task/6337023629328384
[2] https://cirrus-ci.com/task/4929648745775104
[3] https://cirrus-ci.com/task/5570413708705792
[4] https://cirrus-ci.com/task/5409537722679296

-- 
Regards,
Nazir Bilal Yavuz
Microsoft

  13. Re: [PATCH] Fix severe performance regression with gettext 0.20+ on Windows

    Peter Eisentraut <peter@eisentraut.org> — 2025-12-17T09:54:26Z

    On 10.12.25 01:45, Bryan Green wrote:
> The attached patch takes a pragmatic approach: for gettext 0.20.1+, we
> avoid triggering the bug by using Windows locale format instead of
> calling IsoLocaleName(). This works because gettext 0.20.1+ internally
> converts the Windows format back to POSIX for catalog lookups, whereas
> 0.19.8 and earlier need POSIX format directly.

A few comments on the patch itself:

1) The description from the commit message, 'gettext 0.20.1+ expects 
Windows locale format ("English_United States") not POSIX format 
("en_US")', or similar, should be added as a comment to the code.

2) Is the cutoff actually 0.20.1 or should it be 0.20?

3) Similarly some comment about why "C" and "POSIX" are handled specially.

4) There is already earlier in pg_perm_setlocale() code that handles 
LC_MESSAGES specially on Windows, and it deals with the (locale == NULL 
|| locale[0] == '\0') case, which is then repeated later on in your 
patch.  I wonder if this could be simplified or combined somehow.

5) With the patch applied, building with a new-enough gettext will leave 
the function IsoLocaleName() unused, which could result in a compiler 
warning.  You could add pg_attribute_unused() to avoid that.