Horton
Menu

Re: ICU for global collation

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

View thread
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: m.polyakova@postgrespro.ru
Cc: pgsql-hackers@postgresql.org, pryzby@telsasoft.com, rjuju123@gmail.com, daniel@manitou-mail.org, AndrewBille@gmail.com, michael@paquier.xyz, peter.eisentraut@enterprisedb.com
Date: 2022-09-16T08:11:16Z
Lists: pgsql-hackers 
At Fri, 16 Sep 2022 09:49:28 +0300, Marina Polyakova <m.polyakova@postgrespro.ru> wrote in 
> On 2022-09-15 09:52, Kyotaro Horiguchi wrote:
> > However, when I reran the command, it complains about incompatible
> > encoding this time.  I think it's more user-friendly to check for the
> > encoding compatibility before the check for missing --icu-locale
> > option.
> > regards.
> 
> In continuation of options check: AFAICS the following checks in
> initdb
> 
> 	if (locale_provider == COLLPROVIDER_ICU)
> 	{
> 		if (!icu_locale)
> 			pg_fatal("ICU locale must be specified");
> 
> 		/*
> 		 * In supported builds, the ICU locale ID will be checked by the
> 		 * backend during post-bootstrap initialization.
> 		 */
> #ifndef USE_ICU
> 		pg_fatal("ICU is not supported in this build");
> #endif
> 	}
> 
> are executed approximately when they are executed in create database
> after getting all the necessary data from the template database:

initdb doesn't work that way, but anyway, I realized that I am
proposing to move that code in setlocales() to the caller function as
the result. I don't think setlocales() is the place for the code
because icu locale has no business with what the function does.  That
being said there's no obvious reason we *need* to move the code out to
its caller.

> if (dblocprovider == COLLPROVIDER_ICU)
> {
> 	/*
> 	 * This would happen if template0 uses the libc provider but the new
> 	 * database uses icu.
> 	 */
> 	if (!dbiculocale)
> 		ereport(ERROR,
> 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> 				 errmsg("ICU locale must be specified")));
> }
> 
> if (dblocprovider == COLLPROVIDER_ICU)
> 	check_icu_locale(dbiculocale);
> 
> But perhaps the check that --icu-locale cannot be specified unless
> locale provider icu is chosen should also be moved here? So all these
> checks will be in one place and it will use the provider from the
> template database (which could be icu):
> 
> $ initdb --locale-provider icu --icu-locale en-US -D data &&
> pg_ctl -D data -l logfile start &&
> createdb --icu-locale ru-RU --template template0 mydb
> ...
> createdb: error: database creation failed: ERROR: ICU locale cannot be
> specified unless locale provider is ICU

And, I realized that this causes bigger churn than I thought. So, I'm
sorry but I withdraw the comment.

Thus the first proposed patch will be more or less the direction we
would go. And the patch looks good to me as a whole.

+					 errmsg("encoding \"%s\" is not supported with ICU provider",

+		pg_log_error("encoding \"%s\" is not supported with ICU provider",
+					 pg_encoding_to_char(encodingid));

I might be wrong, but the messages look wrong to me.  The alternatives
below might work.

"encoding \"%s\" is not supported by ICU"
"encoding \"%s\" cannot be used for/with ICU locales"

+		pg_log_error_hint("Rerun %s and choose a matching combination.",
+						  progname);

This doesn't seem to provide users with useful information.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

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