From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
cc: Marco Atzeri <marco.atzeri@gmail.com>,
        PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Subject: Re: initdb initalization failure for collation "ja_JP"
In-reply-to: <8599.1498250322@sss.pgh.pa.us>
References: <20c74bc3-d6ca-243d-1bbc-12f17fa4fe9a@gmail.com>
 <28708.1497797324@sss.pgh.pa.us> <9154.1497973022@sss.pgh.pa.us>
 <28195.1498172402@sss.pgh.pa.us> <8599.1498250322@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Fri, 23 Jun 2017 16:38:42 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <15142.1498256231.0@sss.pgh.pa.us>
Date: Fri, 23 Jun 2017 18:20:20 -0400
Message-ID: <15254.1498256420@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

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

I wrote:
>> One question that I've got is why the ICU portion refuses to load
>> any entries unless is_encoding_supported_by_icu(GetDatabaseEncoding()).
>> Surely this is completely wrong?  I should think that what we load into
>> pg_collation ought to be independent of template1's encoding, the same
>> as it is for libc collations, and the right place to be making a test
>> like that is where somebody attempts to use an ICU collation.

Pursuant to the second part of that: I checked on what happens if you
try to use an ICU collation in a database with a not-supported-by-ICU
encoding.  We have to cope with that scenario even with the current
(broken IMO) initdb behavior, because even if template1 has a supported
encoding, it's possible to create another database that doesn't.
It does fail more or less cleanly; you get an "encoding "foo" not
supported by ICU" message at runtime (out of get_encoding_name_for_icu).
But that's quite a bit unlike the behavior for libc collations: with
those, you get an error in collation name lookup, along the lines of

collation "en_DK.utf8" for encoding "SQL_ASCII" does not exist

The attached proposed patch makes the behavior for ICU collations the
same, by dint of injecting the is_encoding_supported_by_icu() check
into collation name lookup.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="collation-lookup-fix.patch";
	charset="us-ascii"
Content-ID: <15142.1498256231.2@sss.pgh.pa.us>
Content-Description: collation-lookup-fix.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespac=
e.c
index 64f6fee..029a132 100644
*** a/src/backend/catalog/namespace.c
--- b/src/backend/catalog/namespace.c
*************** OpfamilyIsVisible(Oid opfid)
*** 1915,1923 ****
--- 1915,1974 ----
  }
=20=20
  /*
+  * lookup_collation
+  *		If there's a collation of the given name/namespace, and it works
+  *		with the given encoding, return its OID.  Else return InvalidOid.
+  */
+ static Oid
+ lookup_collation(const char *collname, Oid collnamespace, int32 encoding)
+ {
+ 	Oid			collid;
+ 	HeapTuple	colltup;
+ 	Form_pg_collation collform;
+=20
+ 	/* Check for encoding-specific entry (exact match) */
+ 	collid =3D GetSysCacheOid3(COLLNAMEENCNSP,
+ 							 PointerGetDatum(collname),
+ 							 Int32GetDatum(encoding),
+ 							 ObjectIdGetDatum(collnamespace));
+ 	if (OidIsValid(collid))
+ 		return collid;
+=20
+ 	/*
+ 	 * Check for any-encoding entry.  This takes a bit more work: while libc
+ 	 * collations with collencoding =3D -1 do work with all encodings, ICU
+ 	 * collations only work with certain encodings, so we have to check that
+ 	 * aspect before deciding it's a match.
+ 	 */
+ 	colltup =3D SearchSysCache3(COLLNAMEENCNSP,
+ 							  PointerGetDatum(collname),
+ 							  Int32GetDatum(-1),
+ 							  ObjectIdGetDatum(collnamespace));
+ 	if (!HeapTupleIsValid(colltup))
+ 		return InvalidOid;
+ 	collform =3D (Form_pg_collation) GETSTRUCT(colltup);
+ 	if (collform->collprovider =3D=3D COLLPROVIDER_ICU)
+ 	{
+ 		if (is_encoding_supported_by_icu(encoding))
+ 			collid =3D HeapTupleGetOid(colltup);
+ 		else
+ 			collid =3D InvalidOid;
+ 	}
+ 	else
+ 	{
+ 		collid =3D HeapTupleGetOid(colltup);
+ 	}
+ 	ReleaseSysCache(colltup);
+ 	return collid;
+ }
+=20
+ /*
   * CollationGetCollid
   *		Try to resolve an unqualified collation name.
   *		Returns OID if collation found in search path, else InvalidOid.
+  *
+  * Note that this will only find collations that work with the current
+  * database's encoding.
   */
  Oid
  CollationGetCollid(const char *collname)
*************** CollationGetCollid(const char *collname)
*** 1935,1953 ****
  		if (namespaceId =3D=3D myTempNamespace)
  			continue;			/* do not look in temp namespace */
=20=20
! 		/* Check for database-encoding-specific entry */
! 		collid =3D GetSysCacheOid3(COLLNAMEENCNSP,
! 								 PointerGetDatum(collname),
! 								 Int32GetDatum(dbencoding),
! 								 ObjectIdGetDatum(namespaceId));
! 		if (OidIsValid(collid))
! 			return collid;
!=20
! 		/* Check for any-encoding entry */
! 		collid =3D GetSysCacheOid3(COLLNAMEENCNSP,
! 								 PointerGetDatum(collname),
! 								 Int32GetDatum(-1),
! 								 ObjectIdGetDatum(namespaceId));
  		if (OidIsValid(collid))
  			return collid;
  	}
--- 1986,1992 ----
  		if (namespaceId =3D=3D myTempNamespace)
  			continue;			/* do not look in temp namespace */
=20=20
! 		collid =3D lookup_collation(collname, namespaceId, dbencoding);
  		if (OidIsValid(collid))
  			return collid;
  	}
*************** CollationGetCollid(const char *collname)
*** 1961,1966 ****
--- 2000,2008 ----
   *		Determine whether a collation (identified by OID) is visible in the
   *		current search path.  Visible means "would be found by searching
   *		for the unqualified collation name".
+  *
+  * Note that only collations that work with the current database's encodi=
ng
+  * will be considered visible.
   */
  bool
  CollationIsVisible(Oid collid)
*************** CollationIsVisible(Oid collid)
*** 1990,1998 ****
  	{
  		/*
  		 * If it is in the path, it might still not be visible; it could be
! 		 * hidden by another conversion of the same name earlier in the path.
! 		 * So we must do a slow check to see if this conversion would be found
! 		 * by CollationGetCollid.
  		 */
  		char	   *collname =3D NameStr(collform->collname);
=20=20
--- 2032,2041 ----
  	{
  		/*
  		 * If it is in the path, it might still not be visible; it could be
! 		 * hidden by another collation of the same name earlier in the path,
! 		 * or it might not work with the current DB encoding.  So we must do a
! 		 * slow check to see if this collation would be found by
! 		 * CollationGetCollid.
  		 */
  		char	   *collname =3D NameStr(collform->collname);
=20=20
*************** PopOverrideSearchPath(void)
*** 3442,3447 ****
--- 3485,3493 ----
=20=20
  /*
   * get_collation_oid - find a collation by possibly qualified name
+  *
+  * Note that this will only find collations that work with the current
+  * database's encoding.
   */
  Oid
  get_collation_oid(List *name, bool missing_ok)
*************** get_collation_oid(List *name, bool missi
*** 3463,3479 ****
  		if (missing_ok && !OidIsValid(namespaceId))
  			return InvalidOid;
=20=20
! 		/* first try for encoding-specific entry, then any-encoding */
! 		colloid =3D GetSysCacheOid3(COLLNAMEENCNSP,
! 								  PointerGetDatum(collation_name),
! 								  Int32GetDatum(dbencoding),
! 								  ObjectIdGetDatum(namespaceId));
! 		if (OidIsValid(colloid))
! 			return colloid;
! 		colloid =3D GetSysCacheOid3(COLLNAMEENCNSP,
! 								  PointerGetDatum(collation_name),
! 								  Int32GetDatum(-1),
! 								  ObjectIdGetDatum(namespaceId));
  		if (OidIsValid(colloid))
  			return colloid;
  	}
--- 3509,3515 ----
  		if (missing_ok && !OidIsValid(namespaceId))
  			return InvalidOid;
=20=20
! 		colloid =3D lookup_collation(collation_name, namespaceId, dbencoding);
  		if (OidIsValid(colloid))
  			return colloid;
  	}
*************** get_collation_oid(List *name, bool missi
*** 3489,3504 ****
  			if (namespaceId =3D=3D myTempNamespace)
  				continue;		/* do not look in temp namespace */
=20=20
! 			colloid =3D GetSysCacheOid3(COLLNAMEENCNSP,
! 									  PointerGetDatum(collation_name),
! 									  Int32GetDatum(dbencoding),
! 									  ObjectIdGetDatum(namespaceId));
! 			if (OidIsValid(colloid))
! 				return colloid;
! 			colloid =3D GetSysCacheOid3(COLLNAMEENCNSP,
! 									  PointerGetDatum(collation_name),
! 									  Int32GetDatum(-1),
! 									  ObjectIdGetDatum(namespaceId));
  			if (OidIsValid(colloid))
  				return colloid;
  		}
--- 3525,3531 ----
  			if (namespaceId =3D=3D myTempNamespace)
  				continue;		/* do not look in temp namespace */
=20=20
! 			colloid =3D lookup_collation(collation_name, namespaceId, dbencoding);
  			if (OidIsValid(colloid))
  				return colloid;
  		}

------- =_aaaaaaaaaa0
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

------- =_aaaaaaaaaa0--