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.


Reply via email to