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