Re: [HACKERS] initdb initalization failure for collation "ja_JP"

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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers