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
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 64f6fee..029a132 100644
*** a/src/backend/catalog/namespace.c
--- b/src/backend/catalog/namespace.c
*************** OpfamilyIsVisible(Oid opfid)
*** 1915,1923 ****
--- 1915,1974 ----
}
/*
+ * 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;
+
+ /* Check for encoding-specific entry (exact match) */
+ collid = GetSysCacheOid3(COLLNAMEENCNSP,
+ PointerGetDatum(collname),
+ Int32GetDatum(encoding),
+ ObjectIdGetDatum(collnamespace));
+ if (OidIsValid(collid))
+ return collid;
+
+ /*
+ * Check for any-encoding entry. This takes a bit more work: while libc
+ * collations with collencoding = -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 = SearchSysCache3(COLLNAMEENCNSP,
+ PointerGetDatum(collname),
+ Int32GetDatum(-1),
+ ObjectIdGetDatum(collnamespace));
+ if (!HeapTupleIsValid(colltup))
+ return InvalidOid;
+ collform = (Form_pg_collation) GETSTRUCT(colltup);
+ if (collform->collprovider == COLLPROVIDER_ICU)
+ {
+ if (is_encoding_supported_by_icu(encoding))
+ collid = HeapTupleGetOid(colltup);
+ else
+ collid = InvalidOid;
+ }
+ else
+ {
+ collid = HeapTupleGetOid(colltup);
+ }
+ ReleaseSysCache(colltup);
+ return collid;
+ }
+
+ /*
* 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 == myTempNamespace)
continue; /* do not look in temp namespace */
! /* Check for database-encoding-specific entry */
! collid = GetSysCacheOid3(COLLNAMEENCNSP,
! PointerGetDatum(collname),
! Int32GetDatum(dbencoding),
! ObjectIdGetDatum(namespaceId));
! if (OidIsValid(collid))
! return collid;
!
! /* Check for any-encoding entry */
! collid = GetSysCacheOid3(COLLNAMEENCNSP,
! PointerGetDatum(collname),
! Int32GetDatum(-1),
! ObjectIdGetDatum(namespaceId));
if (OidIsValid(collid))
return collid;
}
--- 1986,1992 ----
if (namespaceId == myTempNamespace)
continue; /* do not look in temp namespace */
! collid = 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 encoding
+ * 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 = NameStr(collform->collname);
--- 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 = NameStr(collform->collname);
*************** PopOverrideSearchPath(void)
*** 3442,3447 ****
--- 3485,3493 ----
/*
* 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;
! /* first try for encoding-specific entry, then any-encoding */
! colloid = GetSysCacheOid3(COLLNAMEENCNSP,
! PointerGetDatum(collation_name),
! Int32GetDatum(dbencoding),
! ObjectIdGetDatum(namespaceId));
! if (OidIsValid(colloid))
! return colloid;
! colloid = GetSysCacheOid3(COLLNAMEENCNSP,
! PointerGetDatum(collation_name),
! Int32GetDatum(-1),
! ObjectIdGetDatum(namespaceId));
if (OidIsValid(colloid))
return colloid;
}
--- 3509,3515 ----
if (missing_ok && !OidIsValid(namespaceId))
return InvalidOid;
! colloid = lookup_collation(collation_name, namespaceId, dbencoding);
if (OidIsValid(colloid))
return colloid;
}
*************** get_collation_oid(List *name, bool missi
*** 3489,3504 ****
if (namespaceId == myTempNamespace)
continue; /* do not look in temp namespace */
! colloid = GetSysCacheOid3(COLLNAMEENCNSP,
! PointerGetDatum(collation_name),
! Int32GetDatum(dbencoding),
! ObjectIdGetDatum(namespaceId));
! if (OidIsValid(colloid))
! return colloid;
! colloid = GetSysCacheOid3(COLLNAMEENCNSP,
! PointerGetDatum(collation_name),
! Int32GetDatum(-1),
! ObjectIdGetDatum(namespaceId));
if (OidIsValid(colloid))
return colloid;
}
--- 3525,3531 ----
if (namespaceId == myTempNamespace)
continue; /* do not look in temp namespace */
! colloid = lookup_collation(collation_name, namespaceId, dbencoding);
if (OidIsValid(colloid))
return colloid;
}
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
