MIME-Version: 1.0
Date: Sat, 17 Sep 2022 10:05:32 +0300
From: Marina Polyakova <m.polyakova@postgrespro.ru>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
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
Subject: Re: ICU for global collation
In-Reply-To: <20220916.171116.507864395269358149.horikyota.ntt@gmail.com>
References: <6dd6db0984d86a51b7255ba79f111971@postgrespro.ru>
 <20220915.155210.529619933474389138.horikyota.ntt@gmail.com>
 <a739cd973cd3c911d9bf582b6febba1c@postgrespro.ru>
 <20220916.171116.507864395269358149.horikyota.ntt@gmail.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <897b08202324ea098443bf9c3231b9b2@postgrespro.ru>
Organization: Postgres Professional
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/897b08202324ea098443bf9c3231b9b2%40postgrespro.ru>
Precedence: bulk

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