MIME-Version: 1.0
Date: Mon, 22 Aug 2022 17:17:31 +0300
From: Marina Polyakova <m.polyakova@postgrespro.ru>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Cc: pgsql-hackers@postgresql.org, Michael Paquier <michael@paquier.xyz>,
 Julien Rouhaud <rjuju123@gmail.com>, Daniel Verite
 <daniel@manitou-mail.org>, pryzby@telsasoft.com
Subject: Re: ICU for global collation
In-Reply-To: <7d74b2d7-7fc6-f55b-3f77-c0e05569b64e@enterprisedb.com>
References: <20220121135119.ywszwl4353v5dzh5@jrouhaud>
 <e2061949-f731-fd60-8b0b-3bae75095b4f@enterprisedb.com>
 <20220121161331.xn7fhtfg5j2ucilb@jrouhaud>
 <87e93704-b7f7-71f6-161d-8713e6334ed3@enterprisedb.com>
 <704405ed-f1fa-d2a7-b9f9-48a07fb45648@enterprisedb.com>
 <20220625151930.GH22452@telsasoft.com>
 <20220626035124.t36putp232nfl5im@jrouhaud>
 <54b7bd53-ae10-6258-21e6-b799f7480803@enterprisedb.com>
 <YrlRS847pNnxwu+v@paquier.xyz>
 <97f2c29e-24d9-3660-60cf-9138c0c47d90@enterprisedb.com>
 <20220628013205.GH19662@telsasoft.com>
 <f385ba25e7f8be427b8c582e5cca7d79@postgrespro.ru>
 <7d74b2d7-7fc6-f55b-3f77-c0e05569b64e@enterprisedb.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <05d4cba9d1a981fa3ea7eed187fe482b@postgrespro.ru>
Organization: Postgres Professional
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/05d4cba9d1a981fa3ea7eed187fe482b%40postgrespro.ru>
Precedence: bulk

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