Re: ICU for global collation

From: Marina Polyakova <m(dot)polyakova(at)postgrespro(dot)ru>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, pryzby(at)telsasoft(dot)com, rjuju123(at)gmail(dot)com, daniel(at)manitou-mail(dot)org, AndrewBille(at)gmail(dot)com, michael(at)paquier(dot)xyz, peter(dot)eisentraut(at)enterprisedb(dot)com
Subject: Re: ICU for global collation
Date: 2022-09-17 07:05:32
Message-ID: 897b08202324ea098443bf9c3231b9b2@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2022-09-16 11:11, Kyotaro Horiguchi wrote:
> At Fri, 16 Sep 2022 09:49:28 +0300, Marina Polyakova
> <m(dot)polyakova(at)postgrespro(dot)ru> wrote in
>> 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.

Excuse me, but could you explain your last sentence in more detail? I
read that this code is not for setlocales and then - that it should not
moved from here, so I'm confused...

> + 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"

The message indicates that the selected encoding cannot be used with the
ICU provider because it does not support it. But if the text of the
message becomes better and clearer, I will only be glad.

> + pg_log_error_hint("Rerun %s and choose a matching combination.",
> + progname);
>
> This doesn't seem to provide users with useful information.

It was commited in more verbose form:

pg_log_error_hint("Rerun %s and either do not specify an encoding
explicitly, "
"or choose a matching combination.",

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2022-09-17 07:18:34 Re: Refactor backup related code (was: Is it correct to say, "invalid data in file \"%s\"", BACKUP_LABEL_FILE in do_pg_backup_stop?)
Previous Message Michael Paquier 2022-09-17 06:59:13 Re: Making C function declaration parameter names consistent with corresponding definition names