Re: PG compilation error with Visual Studio 2015/2017/2019 - Mailing list pgsql-hackers

On Tue, Apr 21, 2020 at 12:41 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Apr 21, 2020 at 12:51 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> I have tried a simple test with the latest patch and it failed for me.
>
> Set LC_MESSAGES="English_United Kingdom";
> -- returns en-GB, then code changes it to en_NZ when _create_locale()
> is used whereas with the patch it returns "" (empty string).
>
> There seem to be two problems here (a) The input to enum_locales_fn
> doesn't seem to get the input name as "English_United Kingdom" due to
> which it can't find a match even if the same exists. (b) After
> executing EnumSystemLocalesEx, there is no way the patch can detect if
> it is successful in finding the passed name due to which it appends
> empty string in such cases.

Few more comments:
1. I have tried the first one in the list provided by you and that
also didn't work. Basically, I got  empty string when I tried Set
LC_MESSAGES='Afar';

I cannot reproduce any of these errors on my end. When using _create_locale(),  returning "en_NZ" is also a wrong result. 

  

2. Getting below warning
pg_locale.c(1072): warning C4133: 'function': incompatible types -
from 'const char *' to 'const wchar_t *'

Yes, that is a regression.

 

3.
+ if (GetLocaleInfoEx(pStr, LOCALE_SENGLISHCOUNTRYNAME,
+ test_locale + len, LOCALE_NAME_MAX_LENGTH - len) > 0)

All > or <= 0 checks should be changed to "!" types which mean to
check whether the call toGetLocaleInfoEx is success or not.

MSVC does not recommend "!" in all cases, but GetLocaleInfoEx() looks fine, so agreed.

4. In the patch, first, we try to get with LCType as LOCALE_SNAME and
then with LOCALE_SENGLISHLANGUAGENAME and LOCALE_SENGLISHCOUNTRYNAME.
I think we should add comments indicating why we try to get the locale
information with three LCTypes and why the specific order of trying
those types is required.

Agreed.

 

5. In one of the previous emails, you asked whether we have a list of
supported locales.  I don't find any such list. I think it depends on
Windows locales for which you can get the information from
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-lcid/a9eac961-e77d-41a6-90a5-ce1a8b0cdb9c

Yes, that is the information we get from EnumSystemLocalesEx(), without the additional entries _create_locale() has.

Please find attached a new version addressing the above mentioned, and so adding a debug message for trying to get more information on the failed cases.