Re: ICU for global collation

On 2022-08-22 17:10, Peter Eisentraut wrote:
> 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.

Thank you!

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