From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
cc: andrewbille@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17170: Invalid collation created with provider icu and
 initdb' locale C
In-reply-to: <1689869.1630427645@sss.pgh.pa.us>
References: <17170-95845cf3f0a9c36d@postgresql.org>
 <1689869.1630427645@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Tue, 31 Aug 2021 12:34:05 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <2020232.1630440589.0@sss.pgh.pa.us>
Date: Tue, 31 Aug 2021 16:10:53 -0400
Message-ID: <2020287.1630440653@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2020287.1630440653%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2020232.1630440589.1@sss.pgh.pa.us>

I wrote:
> The immediate issue is that DROP COLLATION uses the same lookup
> rule as other code paths, so it fails to see inapplicable
> pg_collation entries at all.  So one approach to a fix is to
> relax its lookup rule.  But I wonder if we should have prevented
> the CREATE in the first place, instead.

After a bit of experimentation, I'm inclined to the idea that
we should have rejected the CREATE COLLATION in the first place.
It's not very useful to create a collation that you're not going
to be able to use, and I think it's more user-friendly to say so
up front than to leave the user guessing about why subsequent
references don't work.

There is arguably one use-case that this fix shuts off: manually
adding ICU-based collations to template0, in the expectation
that they could get cloned into other databases where they'd be
useful.  But that seems like a mighty thin argument, especially
since you could initdb with an ICU-compatible encoding if you
want to do that.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="prevent-creating-useless-ICU-collations.patch";
	charset="us-ascii"
Content-ID: <2020232.1630440589.2@sss.pgh.pa.us>
Content-Description: prevent-creating-useless-ICU-collations.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/c=
ollationcmds.c
index 5a2ba56cec..9b8a259eb2 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -215,7 +215,19 @@ DefineCollation(ParseState *pstate, List *names, List=
 *parameters, bool if_not_e
 	if (!fromEl)
 	{
 		if (collprovider =3D=3D COLLPROVIDER_ICU)
+		{
+			/*
+			 * While an ICU collation can support various encodings, only
+			 * allow one to be created when the current database's encoding is
+			 * supported.  Otherwise we get surprising behaviors like not
+			 * being able to drop the collation.
+			 */
+			if (!is_encoding_supported_by_icu(GetDatabaseEncoding()))
+				ereport(ERROR,
+						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+						 errmsg("current database's encoding is not supported with this pro=
vider")));
 			collencoding =3D -1;
+		}
 		else
 		{
 			collencoding =3D GetDatabaseEncoding();

------- =_aaaaaaaaaa0--