Re: ICU for global collation

Marina Polyakova <m.polyakova@postgrespro.ru>

From: Marina Polyakova <m.polyakova@postgrespro.ru>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Julien Rouhaud <rjuju123@gmail.com>, Daniel Verite <daniel@manitou-mail.org>, AndrewBille@gmail.com, michael@paquier.xyz
Date: 2022-09-13T05:34:16Z
Lists: pgsql-hackers
On 2022-09-09 19:46, Justin Pryzby wrote:
> In pg14:
> |postgres=# create database a LC_COLLATE C LC_CTYPE C LOCALE C;
> |ERROR:  conflicting or redundant options
> |DETAIL:  LOCALE cannot be specified together with LC_COLLATE or 
> LC_CTYPE.
> 
> In pg15:
> |postgres=# create database a LC_COLLATE "en_US.UTF-8" LC_CTYPE
> "en_US.UTF-8" LOCALE "en_US.UTF-8" ;
> |CREATE DATABASE
> 
> f2553d430 actually relaxed the restriction by removing this check:
> 
> -       if (dlocale && (dcollate || dctype))
> -               ereport(ERROR,
> -                               (errcode(ERRCODE_SYNTAX_ERROR),
> -                                errmsg("conflicting or redundant 
> options"),
> -                                errdetail("LOCALE cannot be specified
> together with LC_COLLATE or LC_CTYPE.")));
> 
> But isn't the right fix to do the corresponding thing in createdb
> (relaxing the frontend restriction rather than reverting its relaxation
> in the backend).
> 
> diff --git a/src/bin/scripts/createdb.c b/src/bin/scripts/createdb.c
> index e523e58b218..5b80e56dfd9 100644
> --- a/src/bin/scripts/createdb.c
> +++ b/src/bin/scripts/createdb.c
> @@ -159,15 +159,10 @@ main(int argc, char *argv[])
>  			exit(1);
>  	}
> 
> -	if (locale)
> -	{
> -		if (lc_ctype)
> -			pg_fatal("only one of --locale and --lc-ctype can be specified");
> -		if (lc_collate)
> -			pg_fatal("only one of --locale and --lc-collate can be specified");
> +	if (locale && !lc_ctype)
>  		lc_ctype = locale;
> +	if (locale && !lc_collate)
>  		lc_collate = locale;
> -	}
> 
>  	if (encoding)
>  	{

I agree with you that it is more comfortable and more similar to what 
has already been done in initdb. IMO it would be easier to do it like 
this:

diff --git a/src/bin/scripts/createdb.c b/src/bin/scripts/createdb.c
index 
e523e58b2189275dc603a06324a2f28b0f49d8b7..a1482df3d981a680dd3322052e7c03ddacc8dc26 
100644
--- a/src/bin/scripts/createdb.c
+++ b/src/bin/scripts/createdb.c
@@ -161,12 +161,10 @@ main(int argc, char *argv[])

  	if (locale)
  	{
-		if (lc_ctype)
-			pg_fatal("only one of --locale and --lc-ctype can be specified");
-		if (lc_collate)
-			pg_fatal("only one of --locale and --lc-collate can be specified");
-		lc_ctype = locale;
-		lc_collate = locale;
+		if (!lc_ctype)
+			lc_ctype = locale;
+		if (!lc_collate)
+			lc_collate = locale;
  	}

  	if (encoding)

Should we change the behaviour of createdb and CREATE DATABASE in 
previous major versions?..

> BTW it's somewhat crummy that it uses a string comparison, so if you
> write "UTF8" without a dash, it says this; it took me a few minutes to
> see the difference...
> 
> postgres=# create database a LC_COLLATE "en_US.UTF8" LC_CTYPE
> "en_US.UTF8" LOCALE "en_US.UTF8";
> ERROR:  new collation (en_US.UTF8) is incompatible with the collation
> of the template database (en_US.UTF-8)

Perhaps we could check the locale itself with the function 
normalize_libc_locale_name (collationcmds.c). But ISTM that the current 
check is a safety net in case the function pg_get_encoding_from_locale 
(chklocale.c) returns -1 or PG_SQL_ASCII...

-- 
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Add CHECK_FOR_INTERRUPTS while restoring changes during decoding.

  2. Improve ICU option handling in CREATE DATABASE

  3. Don't allow creation of database with ICU locale with unsupported encoding

  4. Make locale option behavior more consistent

  5. pg_dump: Dump colliculocale

  6. Remove further unwanted linker flags from perl_embed_ldflags

  7. Doc: document possible need to raise kernel's somaxconn limit.

  8. Reduce warnings with -Wshadow=compatible-local builds

  9. Remove redundant spaces in _outA_Expr() output

  10. Fix outdated --help message for postgres -f

  11. pg_upgrade: Fix version comparison for global ICU support

  12. Silence -Wmaybe-uninitialized compiler warning in dbcommands.c.

  13. Add option to use ICU as global locale provider

  14. DefineCollation() code cleanup

  15. Change collate and ctype fields to type text

  16. Call pg_newlocale_from_collation() also with default collation

  17. Use libc version as a collation version on glibc systems.

  18. Silence -Wmaybe-uninitialized compiler warnings in dbcommands.c.

  19. Make LC_COLLATE and LC_CTYPE database-level settings. Collation and