Re: [HACKERS] [GENERAL] Not able to create collation on Windows - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [HACKERS] [GENERAL] Not able to create collation on Windows
Date
Msg-id 14439.1501599195@sss.pgh.pa.us
Whole thread Raw
Responses Re: [HACKERS] [GENERAL] Not able to create collation on Windows  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
List pgsql-hackers
Murtuza Zabuawala <murtuza.zabuawala@enterprisedb.com> writes:
> I am trying to create collation on windows using default POSIX collation
> with pgAdmin3 but I am getting error as shown in screenshot, Can someone
> suggest how to fix this?

> *Syntax:*
> CREATE COLLATION public.test from pg_catalog."POSIX";

> *Error:*
> ERROR: could not create locale "POSIX". No error

Hmm.  Evidently Windows' _create_locale() doesn't accept "POSIX".
You might find that "C" works instead, don't know for sure.

I think this is actually a bug, because the collations code clearly
means to allow clones of the C/POSIX locales --- see eg lc_collate_is_c,
which could be noticeably simpler if that case weren't contemplated.
However, DefineCollation checks validity of the new collation by
unconditionally calling pg_newlocale_from_collation().  That violates
the advice in pg_newlocale_from_collation's header comment:

 * Also, callers should avoid calling this before going down a C/POSIX
 * fastpath, because such a fastpath should work even on platforms without
 * locale_t support in the C library.

Every other call site honors that.

So I think what we ought to do is change DefineCollation more or
less like this:

-    (void) pg_newlocale_from_collation(newoid);
+    if (!lc_collate_is_c(newoid) || !lc_ctype_is_c(newoid))
+        (void) pg_newlocale_from_collation(newoid);

Another issue exposed by this report is that we aren't reporting
_create_locale() failures in a useful way.  It's possible this
could be improved by inserting

#ifdef WIN32
    _dosmaperr(GetLastError());
#endif

into report_newlocale_failure in pg_locale.c, but I'm not really
sure.  Microsoft's man page for _create_locale() fails to say much
of anything about its error-case behavior, and definitely does not
say that it sets the GetLastError indicator.  Still, there's certainly
no chance that printing errno without doing this will be useful.
I would suggest that we do that for starters, and if we hear that
we're still getting silly errors, just hot-wire the code to assume
ENOENT on Windows.

            regards, tom lane


pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: [HACKERS] How to run PG TAP tests on windows?
Next
From: Peter Eisentraut
Date:
Subject: Re: [HACKERS] PostgreSQL 10 (latest beta) and older ICU