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!
