From: Shinoda, Noriyoshi (PN Japan FSIP) <noriyoshi.shinoda@hpe.com>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Daniel Verite <daniel@manitou-mail.org>
Subject: RE: ICU for global collation
Date: Thu, 17 Mar 2022 12:01:14 +0000
Message-ID: <PH7PR84MB1885646854837E4E7EC339BFEE129@PH7PR84MB1885.NAMPRD84.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <7c9270a4-db00-004b-df1a-fede4697a779@enterprisedb.com>
References: <171c5f7c-43ee-dbe9-31f9-5e3eb06a54cf@enterprisedb.com>
<e381e36c-c7a2-2ec0-dbb4-da1dab3f0941@enterprisedb.com>
<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>
<96a50803-72ba-8eec-948a-d87608ac15a0@enterprisedb.com>
<20220305083830.lpz3k3yku5lmm5xs@jrouhaud>
<42ce518d-1426-fa9e-1fcc-de090cd55331@enterprisedb.com>
<20220315070727.xf4sateg3apjwnro@jrouhaud>
<7c9270a4-db00-004b-df1a-fede4697a779@enterprisedb.com>
Hi,
Thank you to all the developers.
I found that the description of the pg_database.daticulocale column was not written in the documentation.
The attached small patch adds a description of the daticulocale column to catalogs.sgml.
Regards,
Noriyoshi Shinoda
-----Original Message-----
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Sent: Thursday, March 17, 2022 7:29 PM
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>; Daniel Verite <daniel@manitou-mail.org>
Subject: Re: ICU for global collation
On 15.03.22 08:41, Julien Rouhaud wrote:
>>>> The locale object in ICU is an identifier that specifies a
>>>> particular locale and has fields for language, country, and an
>>>> optional code to specify further variants or subdivisions. These
>>>> fields also can be represented as a string with the fields separated by an underscore.
>>
>> I think the Localization chapter needs to be reorganized a bit, but
>> I'll leave that for a separate patch.
>
> WFM.
I ended up writing a bit of content for that chapter.
>>> While on that topic, the doc should probably mention that default
>>> ICU collations can only be deterministic.
>>
>> Well, there is no option to do otherwise, so I'm not sure where/how
>> to mention that. We usually don't document options that don't exist.
>> ;-)
>
> Sure, but I'm afraid that users may still be tempted to use ICU
> locales like
> und-u-ks-level2 from the case_insensitive example in the doc and hope
> that it will work accordingly. Or maybe it's just me that still sees
> ICU as dark magic and want to be extra cautious.
I have added this to the CREATE DATABASE ref page.
> Unrelated, but I just realized that we have PGC_INTERNAL gucs for
> lc_ctype and lc_collate. Should we add one for icu_locale too?
I'm not sure. I think the existing ones are more for backward compatibility with the time before it was settable per database.
>>> in AlterCollation(), pg_collation_actual_version(),
>>> AlterDatabaseRefreshColl() and pg_database_collation_actual_version():
>>>
>>> - datum = SysCacheGetAttr(COLLOID, tup, Anum_pg_collation_collcollate, &isnull);
>>> - Assert(!isnull);
>>> - newversion = get_collation_actual_version(collForm->collprovider, TextDatumGetCString(datum));
>>> + datum = SysCacheGetAttr(COLLOID, tup, collForm->collprovider == COLLPROVIDER_ICU ? Anum_pg_collation_colliculocale : Anum_pg_collation_collcollate, &isnull);
>>> + if (!isnull)
>>> + newversion = get_collation_actual_version(collForm->collprovider, TextDatumGetCString(datum));
>>> + else
>>> + newversion = NULL;
>>>
>>> The columns are now nullable, but can you actually end up with a
>>> null locale name in the expected field without manual DML on the
>>> catalog, corruption or similar? I think it should be a plain error
>>> explaining the inconsistency rather then silently assuming there's
>>> no version. Note that at least
>>> pg_newlocale_from_collation() asserts that the specific libc/icu
>>> field it's interested in isn't null.
>>
>> This is required because the default collations's fields are null now.
>
> Yes I saw that, but that's a specific exception. Detecting whether
> it's the DEFAULT_COLLATION_OID or not and raise an error when a null
> value isn't expected seems like it could be worthwhile.
I have fixed that as you suggest.
> So apart from the few details mentioned above I'm happy with this patch!
committed!
Attachments:
[application/octet-stream] pg_database_iculocale_v1.diff (711B, ../PH7PR84MB1885646854837E4E7EC339BFEE129@PH7PR84MB1885.NAMPRD84.PROD.OUTLOOK.COM/2-pg_database_iculocale_v1.diff)
download | inline diff:diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgmlindex bcf2b43..4dc5b34 100644--- a/doc/src/sgml/catalogs.sgml+++ b/doc/src/sgml/catalogs.sgml@@ -3054,6 +3054,15 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<row>
<entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>daticulocale</structfield> <type>text</type>+ </para>+ <para>+ ICU locale ID for this database+ </para></entry>+ </row>++ <row>+ <entry role="catalog_table_entry"><para role="column_definition">
<structfield>datcollversion</structfield> <type>text</type>
</para>
<para>
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: noriyoshi.shinoda@hpe.com, peter.eisentraut@enterprisedb.com, rjuju123@gmail.com, daniel@manitou-mail.org
Subject: RE: ICU for global collation
In-Reply-To: <PH7PR84MB1885646854837E4E7EC339BFEE129@PH7PR84MB1885.NAMPRD84.PROD.OUTLOOK.COM>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox