Subject: Re: Order changes in PG16 since ICU introduction

From: "Daniel Verite" <[email-masked]>
Date: Fri, 26 May 2023 18:24:35 +0200

Jeff Davis wrote:

> > #1
> > 
> > postgres=# create database test1 locale='fr_FR.UTF-8';
> > NOTICE:  using standard form "fr-FR" for ICU locale "fr_FR.UTF-8"
> > ERROR:  new ICU locale (fr-FR) is incompatible with the ICU locale of
> 
> I don't see a problem here. If you specify LOCALE to CREATE DATABASE,
> you should either be using "TEMPLATE template0", or you should be
> expecting an error if the LOCALE doesn't match exactly.
> 
> What would you like to see happen here?

What's odd is that initdb starting in an fr_FR.UTF-8 environment
found that "fr" was the default ICU locale to use, whereas
"create database" reports that "fr" and "fr_FR.UTF-8" refer to
incompatible locales.

To me initdb is wrong when coming up with the less precise "fr"
instead of "fr-FR".

I suggest the attached patch to call uloc_getDefault() instead of the
current code that somehow leaves out the country/region component.

Best regards,
-- 
Daniel Vérité
https://postgresql.verite.pro/
Twitter: @DanielVerite
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 31156e863b..09a5c98cc0 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2354,42 +2354,13 @@ icu_validate_locale(const char *loc_str)
 }
 
 /*
- * Determine default ICU locale by opening the default collator and reading
- * its locale.
- *
- * NB: The default collator (opened using NULL) is different from the collator
- * for the root locale (opened with "", "und", or "root"). The former depends
- * on the environment (useful at initdb time) and the latter does not.
+ * Determine the default ICU locale
  */
 static char *
 default_icu_locale(void)
 {
 #ifdef USE_ICU
-	UCollator  *collator;
-	UErrorCode	status;
-	const char *valid_locale;
-	char	   *default_locale;
-
-	status = U_ZERO_ERROR;
-	collator = ucol_open(NULL, &status);
-	if (U_FAILURE(status))
-		pg_fatal("could not open collator for default locale: %s",
-				 u_errorName(status));
-
-	status = U_ZERO_ERROR;
-	valid_locale = ucol_getLocaleByType(collator, ULOC_VALID_LOCALE,
-										&status);
-	if (U_FAILURE(status))
-	{
-		ucol_close(collator);
-		pg_fatal("could not determine default ICU locale");
-	}
-
-	default_locale = pg_strdup(valid_locale);
-
-	ucol_close(collator);
-
-	return default_locale;
+	return pg_strdup(uloc_getDefault());
 #else
 	pg_fatal("ICU is not supported in this build");
 #endif

---

From: Jeff Davis <[email-masked]>
Date: Fri, 26 May 2023 11:47:02 -0700

> current code that somehow leaves out the country/region component.

Thank you. I experimented with several ICU versions and different
environmental settings, and it does seem better at preserving the name
that comes from the environment.

There is a warning in the docs: "Do not use unless you know what you
are doing."[1] I don't see a reason for the warning or any major risk
from us using it. Perhaps it's because the result is affected by either
the environment or the last uloc_setDefault() call. We don't use
uloc_setDefault(), and we only call uloc_getDefault() once, so I don't
see a risk here.

The fix seems simple enough. Committed.

Regards,
	Jeff Davis

[1]
https://unicode-org.github.io/icu-docs/apidoc/dev/icu4c/uloc_8h.html#a4efa16db7351e62293f8ef0c37aac8d2