Fix global ICU collations for ICU < 54

createdb() didn't check for collation attributes validity, which has
to be done explicitly on ICU < 54.  It also forgot to close the ICU collator
opened during the check which leaks some memory.

To fix both, add a new check_icu_locale() that does all the appropriate
verification and close the ICU collator.

initdb also had some partial check for ICU < 54.  To have consistent error
reporting across major ICU versions, and get rid of the need to include ucol.h,
remove the partial check there.  The backend will report an error if needed
during the post-boostrap iniitialization phase.

Author: Julien Rouhaud <julien.rouhaud@free.fr>
Discussion: https://www.postgresql.org/message-id/20220319041459.qqqiqh335sga5ezj@jrouhaud

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 962e11dd8f4bf5bbc72c6244f19d791948e4fb90..623e5ec77895d161d51a074f6679e42a54f5ad84 100644 (file)
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -458,23 +458,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
    }
 
    if (dblocprovider == COLLPROVIDER_ICU)
-   {
-#ifdef USE_ICU
-       UErrorCode  status;
-
-       status = U_ZERO_ERROR;
-       ucol_open(dbiculocale, &status);
-       if (U_FAILURE(status))
-           ereport(ERROR,
-                   (errmsg("could not open collator for locale \"%s\": %s",
-                           dbiculocale, u_errorName(status))));
-#else
-       ereport(ERROR,
-               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                errmsg("ICU is not supported in this build"), \
-                errhint("You need to rebuild PostgreSQL using %s.", "--with-icu")));
-#endif
-   }
+       check_icu_locale(dbiculocale);
 
    /*
     * Check that the new encoding and locale settings match the source

diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 4019255f8ea4a8a41396e3bf8d9a62f1df21e71b..c84fdd8525e427a50cf9829a3114d7493366707b 100644 (file)
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1985,6 +1985,34 @@ icu_set_collation_attributes(UCollator *collator, const char *loc)
 
 #endif                         /* USE_ICU */
 
+/*
+ * Check if the given locale ID is valid, and ereport(ERROR) if it isn't.
+ */
+void
+check_icu_locale(const char *icu_locale)
+{
+#ifdef USE_ICU
+   UCollator  *collator;
+   UErrorCode  status;
+
+   status = U_ZERO_ERROR;
+   collator = ucol_open(icu_locale, &status);
+   if (U_FAILURE(status))
+       ereport(ERROR,
+               (errmsg("could not open collator for locale \"%s\": %s",
+                       icu_locale, u_errorName(status))));
+
+   if (U_ICU_VERSION_MAJOR_NUM < 54)
+       icu_set_collation_attributes(collator, icu_locale);
+   ucol_close(collator);
+#else
+   ereport(ERROR,
+           (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+            errmsg("ICU is not supported in this build"), \
+            errhint("You need to rebuild PostgreSQL using %s.", "--with-icu")));
+#endif
+}
+
 /*
  * These functions convert from/to libc's wchar_t, *not* pg_wchar_t.
  * Therefore we keep them here rather than with the mbutils code.

diff --git a/src/bin/initdb/Makefile b/src/bin/initdb/Makefile
index 8dd25e7afc6c3826a99293a4c1f2a6ab4904e3bf..b0dd13dfbdfa7b74fcfef7b0215108f2971bf7ea 100644 (file)
--- a/src/bin/initdb/Makefile
+++ b/src/bin/initdb/Makefile
@@ -40,7 +40,7 @@ OBJS = \
 all: initdb
 
 initdb: $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
-   $(CC) $(CFLAGS) $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) $(ICU_LIBS) -o $@$(X)
+   $(CC) $(CFLAGS) $(OBJS) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
 
 # We must pull in localtime.c from src/timezones
 localtime.c: % : $(top_srcdir)/src/timezone/%

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index cbcd55288f263ccc361deaf3e4aa49c54c2561d0..5e36943ef3bf364faf2416128097b9f9386d9d97 100644 (file)
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -55,10 +55,6 @@
 #include <signal.h>
 #include <time.h>
 
-#ifdef USE_ICU
-#include <unicode/ucol.h>
-#endif
-
 #ifdef HAVE_SHM_OPEN
 #include "sys/mman.h"
 #endif
@@ -2205,22 +2201,10 @@ setlocales(void)
        }
 
        /*
-        * Check ICU locale ID
+        * In supported builds, the ICU locale ID will be checked by the
+        * backend when performing the post-boostrap initialization.
         */
-#ifdef USE_ICU
-       {
-           UErrorCode  status;
-
-           status = U_ZERO_ERROR;
-           ucol_open(icu_locale, &status);
-           if (U_FAILURE(status))
-           {
-               pg_log_error("could not open collator for locale \"%s\": %s",
-                            icu_locale, u_errorName(status));
-               exit(1);
-           }
-       }
-#else
+#ifndef USE_ICU
        pg_log_error("ICU is not supported in this build");
        fprintf(stderr, _("You need to rebuild PostgreSQL using %s.\n"), "--with-icu");
        exit(1);

diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index c636bf3ab2c7ed87d6745eb29b460cd3d921425b..a3397777cf256cd67166c608f20efe070d37f764 100644 (file)
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -105,7 +105,7 @@ if ($ENV{with_icu} eq 'yes')
        'option --icu-locale');
 
    command_fails_like(['initdb', '--no-sync', '--locale-provider=icu', '--icu-locale=@colNumeric=lower', "$tempdir/dataX"],
-       qr/initdb: error: could not open collator for locale/,
+       qr/FATAL:  could not open collator for locale/,
        'fails for invalid ICU locale');
 }
 else

diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index 9b158f24a00df395bb4c1a8f857cbba2e280fa16..a44e17ffdf8a39468fad3dad73bf264e052eb786 100644 (file)
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -116,6 +116,7 @@ extern char *get_collation_actual_version(char collprovider, const char *collcol
 extern int32_t icu_to_uchar(UChar **buff_uchar, const char *buff, size_t nbytes);
 extern int32_t icu_from_uchar(char **result, const UChar *buff_uchar, int32_t len_uchar);
 #endif
+extern void check_icu_locale(const char *icu_locale);
 
 /* These functions convert from/to libc's wchar_t, *not* pg_wchar_t */
 extern size_t wchar2char(char *to, const wchar_t *from, size_t tolen,

diff --git a/src/test/icu/t/010_database.pl b/src/test/icu/t/010_database.pl
index d50941b53d24af4e007c634c30ee5e2953beda18..07a1084b09d00d68a190386d7d560faef75a3630 100644 (file)
--- a/src/test/icu/t/010_database.pl
+++ b/src/test/icu/t/010_database.pl
@@ -16,11 +16,11 @@ $node1->init;
 $node1->start;
 
 $node1->safe_psql('postgres',
-   q{CREATE DATABASE dbicu LOCALE_PROVIDER icu LOCALE 'C' ICU_LOCALE 'en-u-kf-upper' ENCODING 'UTF8' TEMPLATE template0});
+   q{CREATE DATABASE dbicu LOCALE_PROVIDER icu LOCALE 'C' ICU_LOCALE 'en@colCaseFirst=upper' ENCODING 'UTF8' TEMPLATE template0});
 
 $node1->safe_psql('dbicu',
 q{
-CREATE COLLATION upperfirst (provider = icu, locale = 'en-u-kf-upper');
+CREATE COLLATION upperfirst (provider = icu, locale = 'en@colCaseFirst=upper');
 CREATE TABLE icu (def text, en text COLLATE "en-x-icu", upfirst text COLLATE upperfirst);
 INSERT INTO icu VALUES ('a', 'a', 'a'), ('b', 'b', 'b'), ('A', 'A', 'A'), ('B', 'B', 'B');
 });