Re: Fix order of checking ICU options in initdb and create database

On 19.11.22 20:36, Марина Полякова wrote:
> Here is another set of proposed patches:
>
> v2-0001-Fix-encoding-check-in-initdb-when-the-option-icu-.patch
> Target: PG 15+
> Fix encoding check in initdb when the option --icu-locale is not used:

I'm having a hard time figuring out from your examples what you are
trying to change. Which one is the "before" example and which one is
the "after", and which aspect specifically is the issue and what
specifically is being addressed? I tried out the examples in the
current code and didn't find anything obviously wrong in the behavior.

I'm concerned that the initdb.c changes are a bit unprincipled. They
just move code around to achieve some behavior without keeping the
overall structure in mind. For example, check_icu_locale_encoding()
intentionally had the same API as check_locale_encoding(), but now
that's being changed. And setlocales() did all the locale parameter
validity checking, but now part of that is being moved around. I'm
afraid this makes initdb.c even more spaghetti code than it already is.

What about those test changes? I can't tell if they are related.
createdb isn't being changed; is that test change related or separate?

> v2-0002-doc-building-without-ICU-is-not-recommended.patch
> Target: PG 15+
> Fix the documentation that --without-icu is a marginal configuration.
>
> v2-0003-Build-with-ICU-by-default.patch
> Target: PG 16+
> Build with ICU by default as already done for readline and zlib libraries.

We are not going to make these kinds of changes specifically for ICU.
I'd say, for example, the same applies to --with-openssl and --with-lz4,
and probably others. If this is an issue, then we need a more general
approach than just ICU. This should be a separate thread in any case.