Feedback-ID: i131946ab:Fastmail
Message-ID: <d62b2874-729b-d26a-2d0a-0d64f509eca4@enterprisedb.com>
Date: Thu, 2 Mar 2023 10:37:27 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.8.0
Subject: Re: Move defaults toward ICU in 16?
Content-Language: en-US
To: Jeff Davis <pgsql@j-davis.com>, Andres Freund <andres@anarazel.de>,
 Bruce Momjian <bruce@momjian.us>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Thomas Munro <thomas.munro@gmail.com>,
 Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@postgresql.org
References: <5ffeafe843d43a975f31479fbd211ab88404841b.camel@j-davis.com>
 <20230211020042.uthdgj72kp3xlqam@awork3.anarazel.de>
 <d47923e2774cb5e5b6207f8678ffcc11bd110c4a.camel@j-davis.com>
 <20230214175957.idkb7shsqzp5nbll@awork3.anarazel.de>
 <e4387c6959c9ed0d6dc9f14e01ff191f3e1a30c0.camel@j-davis.com>
 <2a7feb71dbdcea31478b3974b8075982ac2326d2.camel@j-davis.com>
 <20230217170523.ujnq3qr75rolfrmu@awork3.anarazel.de>
 <c412488b2a19467e4c037ad02db5d91431bdf178.camel@j-davis.com>
 <20230217180909.oqxjgjudbsofn666@awork3.anarazel.de>
 <3bf6b5517033947c134ed1ba9a325179a608bfd1.camel@j-davis.com>
 <20230217205009.4sy4lmmqanbhjsgb@awork3.anarazel.de>
 <558f794c351fb73596e4b05d96f66d01d56b4a50.camel@j-davis.com>
 <f2b896d1476915862b99c2514c298c052e048c57.camel@j-davis.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <f2b896d1476915862b99c2514c298c052e048c57.camel@j-davis.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/d62b2874-729b-d26a-2d0a-0d64f509eca4%40enterprisedb.com>
Precedence: bulk

On 25.02.23 00:54, Jeff Davis wrote:
> On Fri, 2023-02-17 at 15:07 -0800, Jeff Davis wrote:
>> 2. Update the pg_database entry for template0. This has less
>> potential
>> for surprise in case people are actually using template0 for a
>> template.
> 
> New patches attached.
> 
>    0001: default autoconf to build with ICU (meson already uses 'auto')

I would skip this.  There was a brief discussion about this at [0], 
where I pointed out that if we are going to do something like that, 
there would be other candidates among the optional dependencies to 
promote, such as certainly openssl and arguably lz4.  If we don't do 
this consistently across dependencies, then there will be confusion.

In practice, I don't think it matters.  Almost all installations are 
made by packagers, who will make their own choices.  Flipping the 
default in configure is only going to cause some amount of confusion and 
annoyance in some places, but won't actually have the ostensibly desired 
impact in practice.

[0]: 
https://www.postgresql.org/message-id/flat/534fed4a262fee534662bd07a691c5ef%40postgrespro.ru

>    0002: update template0 in new cluster (as described above)

This makes sense.  I'm confused what the code originally wanted to 
achieve, e.g.,

-/*
- * Check that every database that already exists in the new cluster is
- * compatible with the corresponding database in the old one.
- */
-static void
-check_databases_are_compatible(void)

Was there once support for the new cluster having additional databases 
in place?  Weird.

In any case, I think you can remove additional code from get_db_infos() 
related to fields that are no longer used, such as db_encoding, and the 
corresponding struct fields in DbInfo.

>    0003: default initdb to use ICU

What are the changes in the citext tests about?  Is it the same issue as 
in unaccent?  In that case, the OR should be an AND?  Maybe add a comment?

Why is unaccent is "broken" if the default collation is provided by ICU 
and LC_CTYPE=C?  Is that a general problem?  Should we prevent this 
combination?

What are the changes in the ecpg tests about?  Looks harmless, but if 
there is a need, maybe it should be commented somewhere, otherwise what 
prevents someone from changing it back?