Re: ICU for global collation

Hello everyone in this thread!

While reading and testing the patch that adds ICU for global collations
[1] I noticed on master (1c5818b9c68e5c2ac8f19d372f24cce409de1a26) and
REL_15_STABLE (63b64d8270691894a9a8f2d4e929e7780020edb8) that:

1) pg_upgrade from REL_14_STABLE
(63b64d8270691894a9a8f2d4e929e7780020edb8) does not always work:

For REL_14_STABLE:

$ initdb -D data_old

For REL_15_STABLE or master:

$ initdb -D data_new --locale-provider icu --icu-locale ru-RU
$ pg_upgrade -d .../data_old -D data_new -b ... -B ...
...
Restoring database schemas in the new cluster
template1
*failure*

Consult the last few lines of
"data_new/pg_upgrade_output.d/20220815T142454.223/log/pg_upgrade_dump_1.log"
for
the probable cause of the failure.
Failure, exiting

In
data_new/pg_upgrade_output.d/20220815T142454.223/log/pg_upgrade_dump_1.log:

pg_restore: error: could not execute query: server closed the connection
unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
Command was: CREATE DATABASE "template1" WITH TEMPLATE = template0 OID =
1 ENCODING = 'UTF8' LOCALE_PROVIDER = libc LOCALE = 'en_US.UTF-8';

In
data_new/pg_upgrade_output.d/20220815T142454.223/log/pg_upgrade_server.log:

TRAP: FailedAssertion("(dblocprovider == COLLPROVIDER_ICU &&
dbiculocale) || (dblocprovider != COLLPROVIDER_ICU && !dbiculocale)",
File: "dbcommands.c", Line: 1292, PID: 69247)
postgres: marina postgres [local] CREATE
DATABASE(ExceptionalCondition+0xb9)[0xb4d8ec]
postgres: marina postgres [local] CREATE
DATABASE(createdb+0x1abc)[0x68ca99]
postgres: marina postgres [local] CREATE
DATABASE(standard_ProcessUtility+0x651)[0x9b1d82]
postgres: marina postgres [local] CREATE
DATABASE(ProcessUtility+0x122)[0x9b172a]
postgres: marina postgres [local] CREATE DATABASE[0x9b01cf]
postgres: marina postgres [local] CREATE DATABASE[0x9b0433]
postgres: marina postgres [local] CREATE
DATABASE(PortalRun+0x2fe)[0x9af95d]
postgres: marina postgres [local] CREATE DATABASE[0x9a953b]
postgres: marina postgres [local] CREATE
DATABASE(PostgresMain+0x733)[0x9ada6b]
postgres: marina postgres [local] CREATE DATABASE[0x8ec632]
postgres: marina postgres [local] CREATE DATABASE[0x8ebfbb]
postgres: marina postgres [local] CREATE DATABASE[0x8e8653]
postgres: marina postgres [local] CREATE
DATABASE(PostmasterMain+0x1226)[0x8e7f26]
postgres: marina postgres [local] CREATE DATABASE[0x7bbccb]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0x7eff082f90b3]
postgres: marina postgres [local] CREATE DATABASE(_start+0x2e)[0x49c29e]
2022-08-15 14:24:56.124 MSK [69231] LOG: server process (PID 69247) was
terminated by signal 6: Aborted
2022-08-15 14:24:56.124 MSK [69231] DETAIL: Failed process was running:
CREATE DATABASE "template1" WITH TEMPLATE = template0 OID = 1 ENCODING =
'UTF8' LOCALE_PROVIDER = libc LOCALE = 'en_US.UTF-8';

1.1) It looks like there's a bug in the function get_db_infos
(src/bin/pg_upgrade/info.c), where the version of the old cluster is
always checked:

if (GET_MAJOR_VERSION(old_cluster.major_version) < 1500)
snprintf(query + strlen(query), sizeof(query) - strlen(query),
"'c' AS datlocprovider, NULL AS daticulocale, ");
else
snprintf(query + strlen(query), sizeof(query) - strlen(query),
"datlocprovider, daticulocale, ");

With the simple patch

diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index
df374ce4b362b4c6c87fc1fd0e476e5d6d353d9e..53ea348e211d3ac38334292bc16cb814bc13bb87
100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -319,7 +319,7 @@ get_db_infos(ClusterInfo *cluster)

snprintf(query, sizeof(query),
"SELECT d.oid, d.datname, d.encoding, d.datcollate, d.datctype, ");
- if (GET_MAJOR_VERSION(old_cluster.major_version) < 1500)
+ if (GET_MAJOR_VERSION(cluster->major_version) < 1500)
snprintf(query + strlen(query), sizeof(query) - strlen(query),
"'c' AS datlocprovider, NULL AS daticulocale, ");
else

I got the expected error during the upgrade:

locale providers for database "template1" do not match: old "libc", new
"icu"
Failure, exiting

1.2) It looks like the mentioned asserion in dbcommands.c conflicts with
the following lines earlier:

if (dbiculocale == NULL)
dbiculocale = src_iculocale;

The following patch works for me:

diff --git a/src/backend/commands/dbcommands.c
b/src/backend/commands/dbcommands.c
index
b31a30550b025d48ba3cc250dc4c15f41f9a80be..17a2942341e528c01182fb6d4878580f2706bec9
100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -1048,6 +1048,8 @@ createdb(ParseState *pstate, const CreatedbStmt
*stmt)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("ICU locale must be specified")));
}
+ else
+ dbiculocale = NULL;

if (dblocprovider == COLLPROVIDER_ICU)
check_icu_locale(dbiculocale);

2) CREATE DATABASE does not always require the icu locale unlike initdb
and createdb:

$ initdb -D data --locale en_US.UTF-8 --locale-provider icu
...
initdb: error: ICU locale must be specified

$ initdb -D data --locale en_US.UTF-8
$ pg_ctl -D data -l logfile start

$ createdb mydb --locale en_US.UTF-8 --template template0
--locale-provider icu
createdb: error: database creation failed: ERROR: ICU locale must be
specified

$ psql -c "CREATE DATABASE mydb LOCALE \"en_US.UTF-8\" TEMPLATE
template0 LOCALE_PROVIDER icu" postgres
CREATE DATABASE

$ psql -c "CREATE DATABASE mydb TEMPLATE template0 LOCALE_PROVIDER icu"
postgres
ERROR: ICU locale must be specified

I'm wondering if this is not a fully-supported feature (because createdb
creates an SQL command with LC_COLLATE and LC_CTYPE options instead of
LOCALE option) or is it a bug in CREATE DATABASE?.. From
src/backend/commands/dbcommands.c:

if (dblocprovider == COLLPROVIDER_ICU && !dbiculocale)
{
if (dlocale && dlocale->arg)
dbiculocale = defGetString(dlocale);
}

[1]
https://github.com/postgres/postgres/commit/f2553d43060edb210b36c63187d52a632448e1d2

--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company