Re: ICU for global collation - Mailing list pgsql-hackers

From Peter Eisentraut
Subject Re: ICU for global collation
Date
Msg-id 7d74b2d7-7fc6-f55b-3f77-c0e05569b64e@enterprisedb.com
Whole thread Raw
In response to Re: ICU for global collation  (Marina Polyakova <m.polyakova@postgrespro.ru>)
Responses Re: ICU for global collation
Re: ICU for global collation
List pgsql-hackers
On 15.08.22 14:06, Marina Polyakova wrote:
> 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

fixed

> 1.2) It looks like the mentioned asserion in dbcommands.c conflicts with 
> the following lines earlier:
> 
> if (dbiculocale == NULL)
>      dbiculocale = src_iculocale;

fixed

> 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);
> }

I think this piece of code was left over from some earlier attempts to 
specify the libc locale and the icu locale with one option, which never 
really worked well.  The CREATE DATABASE man page does not mention that 
LOCALE provides the default for ICU_LOCALE.  Hence, I think we should 
delete this.



pgsql-hackers by date:

Previous
From: Justin Pryzby
Date:
Subject: Re: Schema variables - new implementation for Postgres 15
Next
From: Marina Polyakova
Date:
Subject: Re: ICU for global collation