From 0fb0a0503bfc125764c8dba4f515058145dc7f8b Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 22 Feb 2021 23:01:20 +1300 Subject: [PATCH] Hide internal error for pg_collation_actual_version(). Instead of an unsightly internal "cache lookup failed" message, just return NULL for bad OIDs, as is the convention for other similar things. Reported-by: Justin Pryzby Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/20210117215940.GE8560%40telsasoft.com --- src/backend/catalog/index.c | 5 +++-- src/backend/catalog/pg_depend.c | 3 ++- src/backend/commands/collationcmds.c | 2 +- src/backend/utils/adt/pg_locale.c | 9 +++++++-- src/include/utils/pg_locale.h | 2 +- src/test/regress/expected/collate.icu.utf8.out | 14 ++++++++++++++ src/test/regress/sql/collate.icu.utf8.sql | 5 +++++ 7 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index ea22256819c..4ef61b5efd5 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1290,7 +1290,8 @@ do_collation_version_check(const ObjectAddress *otherObject, return false; /* Ask the provider for the current version. Give up if unsupported. */ - current_version = get_collation_version_for_oid(otherObject->objectId); + current_version = get_collation_version_for_oid(otherObject->objectId, + false); if (!current_version) return false; @@ -1369,7 +1370,7 @@ do_collation_version_update(const ObjectAddress *otherObject, if (OidIsValid(*coll) && otherObject->objectId != *coll) return false; - *new_version = get_collation_version_for_oid(otherObject->objectId); + *new_version = get_collation_version_for_oid(otherObject->objectId, false); return true; } diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 63da24322d9..362db7fe913 100644 --- a/src/backend/catalog/pg_depend.c +++ b/src/backend/catalog/pg_depend.c @@ -116,7 +116,8 @@ recordMultipleDependencies(const ObjectAddress *depender, referenced->objectId == POSIX_COLLATION_OID) continue; - version = get_collation_version_for_oid(referenced->objectId); + version = get_collation_version_for_oid(referenced->objectId, + false); /* * Default collation is pinned, so we need to force recording diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c index 9634ae6809d..a7ee452e192 100644 --- a/src/backend/commands/collationcmds.c +++ b/src/backend/commands/collationcmds.c @@ -273,7 +273,7 @@ pg_collation_actual_version(PG_FUNCTION_ARGS) Oid collid = PG_GETARG_OID(0); char *version; - version = get_collation_version_for_oid(collid); + version = get_collation_version_for_oid(collid, true); if (version) PG_RETURN_TEXT_P(cstring_to_text(version)); diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c index e9c1231f9ba..34b82b9335c 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -1726,10 +1726,11 @@ get_collation_actual_version(char collprovider, const char *collcollate) /* * Get provider-specific collation version string for a given collation OID. * Return NULL if the provider doesn't support versions, or the collation is - * unversioned (for example "C"). + * unversioned (for example "C"). Unknown OIDs result in NULL if missing_ok is + * true. */ char * -get_collation_version_for_oid(Oid oid) +get_collation_version_for_oid(Oid oid, bool missing_ok) { HeapTuple tp; char *version; @@ -1751,7 +1752,11 @@ get_collation_version_for_oid(Oid oid) tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(oid)); if (!HeapTupleIsValid(tp)) + { + if (missing_ok) + return NULL; elog(ERROR, "cache lookup failed for collation %u", oid); + } collform = (Form_pg_collation) GETSTRUCT(tp); version = get_collation_actual_version(collform->collprovider, NameStr(collform->collcollate)); diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h index 34dff74bd11..5a37caefbe4 100644 --- a/src/include/utils/pg_locale.h +++ b/src/include/utils/pg_locale.h @@ -103,7 +103,7 @@ typedef struct pg_locale_struct *pg_locale_t; extern pg_locale_t pg_newlocale_from_collation(Oid collid); -extern char *get_collation_version_for_oid(Oid collid); +extern char *get_collation_version_for_oid(Oid collid, bool missing_ok); #ifdef USE_ICU extern int32_t icu_to_uchar(UChar **buff_uchar, const char *buff, size_t nbytes); diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out index bc3752e9236..de70cb12127 100644 --- a/src/test/regress/expected/collate.icu.utf8.out +++ b/src/test/regress/expected/collate.icu.utf8.out @@ -2155,3 +2155,17 @@ DROP SCHEMA collate_tests CASCADE; RESET client_min_messages; -- leave a collation for pg_upgrade test CREATE COLLATION coll_icu_upgrade FROM "und-x-icu"; +-- Test user-visible function for inspecting versions +SELECT pg_collation_actual_version('"en-x-icu"'::regcollation) is not null; + ?column? +---------- + t +(1 row) + +-- Invalid OIDs are silently ignored +SELECT pg_collation_actual_version(0) is null; + ?column? +---------- + t +(1 row) + diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql index 0de2ed8d856..dd5d2088547 100644 --- a/src/test/regress/sql/collate.icu.utf8.sql +++ b/src/test/regress/sql/collate.icu.utf8.sql @@ -883,3 +883,8 @@ RESET client_min_messages; -- leave a collation for pg_upgrade test CREATE COLLATION coll_icu_upgrade FROM "und-x-icu"; + +-- Test user-visible function for inspecting versions +SELECT pg_collation_actual_version('"en-x-icu"'::regcollation) is not null; +-- Invalid OIDs are silently ignored +SELECT pg_collation_actual_version(0) is null;