From 992608eb81265856af3b9cbcb63597d91876090a Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 17 Feb 2021 14:10:40 +1300 Subject: [PATCH 1/4] Improve error message for pg_collation_actual_version(). Instead of an internal "cache lookup failed" message, show a message intended for end user consumption, considering this is a user-exposed function. Reported-by: Justin Pryzby 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 | 8 +++++++- src/backend/utils/adt/pg_locale.c | 17 +++++++++++++++-- src/include/utils/pg_locale.h | 2 +- 5 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 1514937748..1c808f55c5 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, + NULL); 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, NULL); return true; } diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 63da24322d..aee59eef39 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, + NULL); /* * 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 9634ae6809..b3c59e6e9f 100644 --- a/src/backend/commands/collationcmds.c +++ b/src/backend/commands/collationcmds.c @@ -272,8 +272,14 @@ pg_collation_actual_version(PG_FUNCTION_ARGS) { Oid collid = PG_GETARG_OID(0); char *version; + bool found; - version = get_collation_version_for_oid(collid); + version = get_collation_version_for_oid(collid, &found); + + if (!found) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("no collation found for OID %u", collid))); 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 e9c1231f9b..3cd9257800 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -1726,10 +1726,12 @@ 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"). If "found" is non-NULL, unknown OIDs are + * reported through this output parameter; otherwise, an internal error is + * raised for unknown OIDs. */ char * -get_collation_version_for_oid(Oid oid) +get_collation_version_for_oid(Oid oid, bool *found) { HeapTuple tp; char *version; @@ -1744,6 +1746,8 @@ get_collation_version_for_oid(Oid oid) dbform = (Form_pg_database) GETSTRUCT(tp); version = get_collation_actual_version(COLLPROVIDER_LIBC, NameStr(dbform->datcollate)); + if (found) + *found = true; } else { @@ -1751,10 +1755,19 @@ get_collation_version_for_oid(Oid oid) tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(oid)); if (!HeapTupleIsValid(tp)) + { + if (found) + { + *found = false; + 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)); + if (found) + *found = true; } ReleaseSysCache(tp); diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h index 34dff74bd1..9d73774a19 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 *found); #ifdef USE_ICU extern int32_t icu_to_uchar(UChar **buff_uchar, const char *buff, size_t nbytes); -- 2.30.0