Horton

Re: Built-in CTYPE provider

Peter Eisentraut <peter@eisentraut.org>

View thread View on postgr.es
From: Peter Eisentraut <peter@eisentraut.org>
To: Jeff Davis <pgsql@j-davis.com>, Daniel Verite <daniel@manitou-mail.org>
Cc: Robert Haas <robertmhaas@gmail.com>, Jeremy Schneider <schneider@ardentperf.com>, pgsql-hackers@postgresql.org
Date: 2024-03-14T08:54:35Z
Lists: pgsql-hackers 
On 14.03.24 09:08, Jeff Davis wrote:
> On Wed, 2024-03-13 at 00:44 -0700, Jeff Davis wrote:
>> New series attached. I plan to commit 0001 very soon.
> 
> Committed the basic builtin provider, supporting only the "C" locale.

As you were committing this, I had another review of 
v23-0001-Introduce-collation-provider-builtin.patch in progress.  Some 
of the things I found you have already addressed in what you committed. 
Please check the remaining comments.


* doc/src/sgml/charset.sgml

I don't understand the purpose of this sentence:

"When using this locale, the behavior may depend on the database encoding."


* doc/src/sgml/ref/create_database.sgml

The new parameter builtin_locale is not documented.


* src/backend/commands/collationcmds.c

I think DefineCollation() should set collencoding = -1 for the
COLLPROVIDER_BUILTIN case.  -1 stands for any encoding.  Or at least
explain why not?


* src/backend/utils/adt/pg_locale.c

This part is a bit confusing:

+           cache_entry->collate_is_c = true;
+           cache_entry->ctype_is_c = (strcmp(colllocale, "C") == 0);

Is collate always C but ctype only sometimes?  Does this anticipate
future patches in this series?  Maybe in this patch it should always
be true?


* src/bin/initdb/initdb.c

+   printf(_("      --builtin-locale=LOCALE   set builtin locale name 
for new databases\n"));

Put in a line break so that the right "column" lines up.

This output should line up better:

The database cluster will be initialized with this locale configuration:
   default collation provider:  icu
   default collation locale:    en
   LC_COLLATE:  C
   LC_CTYPE:    C
   ...

Also, why are there two spaces after "provider:  "?

Also we call these locale provider on input, why are they collation
providers on output?  What is a "collation locale"?


* src/bin/pg_upgrade/t/002_pg_upgrade.pl

+if ($oldnode->pg_version >= '17devel')

This is weird.  >= is a numeric comparison, so providing a string with
non-digits is misleading at best.


* src/test/icu/t/010_database.pl

-# Test that LOCALE works for ICU locales if LC_COLLATE and LC_CTYPE
-# are specified

Why remove this test?

+my ($ret, $stdout, $stderr) = $node1->psql('postgres',
+   q{CREATE DATABASE dbicu LOCALE_PROVIDER builtin LOCALE 'C' TEMPLATE 
dbicu}
+);

Change the name of the new database to be different from the name of
the template database.

Commits

  1. Support PG_UNICODE_FAST locale in the builtin collation provider.

  2. Support Unicode full case mapping and conversion.

  3. Fix test failures when language environment is not UTF-8.

  4. Add unicode_strtitle() for Unicode Default Case Conversion.

  5. Use version for builtin collations.

  6. Fix convert_case(), introduced in 5c40364dd6.

  7. Inline basic UTF-8 functions.

  8. Support C.UTF-8 locale in the new builtin collation provider.

  9. Fix another warning, introduced by 846311051e.

  10. Address more review comments on commit 2d819a08a1.

  11. Fix unreachable code warning from commit 2d819a08a1.

  12. Introduce "builtin" collation provider.

  13. Catalog changes preparing for builtin collation provider.

  14. Unicode case mapping tables and functions.

  15. Add Unicode property tables.

  16. Documentation update for Standard Collations.

  17. Cleanup for unicode-update build target and test.

  18. Shrink Unicode category table.

  19. Make some error strings more generic

  20. pg_upgrade: copy locale and encoding information to new cluster.

  21. Update Unicode data to Unicode 15.0.0

  22. Create a new type category for "internal use" types.