From e7941a976688f0f5d13a5227ed4f3efe0718db9d Mon Sep 17 00:00:00 2001 From: Tom Lane <tgl@sss.pgh.pa.us> Date: Sun, 4 Jun 2017 16:20:03 -0400 Subject: [PATCH] Replace over-optimistic Assert in partitioning code with a runtime test. get_partition_parent felt that it could simply Assert that systable_getnext found a tuple. This is unlike any other caller of that function, and it's unsafe IMO --- in fact, the reason I noticed it was that the Assert failed. (OK, I was working with known-inconsistent catalog contents, but I wasn't expecting the DB to fall over quite that violently. The behavior in a non-assert-enabled build wouldn't be very nice, either.) Fix it to do what other callers do, namely an actual runtime-test-and-elog. Also, standardize the wording of elog messages that are complaining about unexpected failure of systable_getnext. 90% of them say "could not find tuple for <object>", so make the remainder do likewise. Many of the holdouts were using the phrasing "cache lookup failed", which is outright misleading since no catcache search is involved. --- contrib/sepgsql/database.c | 2 +- contrib/sepgsql/proc.c | 4 ++-- contrib/sepgsql/relation.c | 8 ++++---- contrib/sepgsql/schema.c | 2 +- src/backend/catalog/aclchk.c | 4 ++-- src/backend/catalog/objectaddress.c | 2 +- src/backend/catalog/partition.c | 3 ++- src/backend/commands/extension.c | 9 +++++---- src/backend/utils/adt/ruleutils.c | 2 +- 9 files changed, 19 insertions(+), 17 deletions(-) diff --git a/contrib/sepgsql/database.c b/contrib/sepgsql/database.c index 69dd290a77..8fc5a87e00 100644 --- a/contrib/sepgsql/database.c +++ b/contrib/sepgsql/database.c @@ -88,7 +88,7 @@ sepgsql_database_post_create(Oid databaseId, const char *dtemplate) SnapshotSelf, 1, &skey); tuple = systable_getnext(sscan); if (!HeapTupleIsValid(tuple)) - elog(ERROR, "catalog lookup failed for database %u", databaseId); + elog(ERROR, "could not find tuple for database %u", databaseId); datForm = (Form_pg_database) GETSTRUCT(tuple); diff --git a/contrib/sepgsql/proc.c b/contrib/sepgsql/proc.c index 4ccf4a5e60..73564edaa7 100644 --- a/contrib/sepgsql/proc.c +++ b/contrib/sepgsql/proc.c @@ -68,7 +68,7 @@ sepgsql_proc_post_create(Oid functionId) tuple = systable_getnext(sscan); if (!HeapTupleIsValid(tuple)) - elog(ERROR, "catalog lookup failed for proc %u", functionId); + elog(ERROR, "could not find tuple for function %u", functionId); proForm = (Form_pg_proc) GETSTRUCT(tuple); @@ -261,7 +261,7 @@ sepgsql_proc_setattr(Oid functionId) SnapshotSelf, 1, &skey); newtup = systable_getnext(sscan); if (!HeapTupleIsValid(newtup)) - elog(ERROR, "catalog lookup failed for function %u", functionId); + elog(ERROR, "could not find tuple for function %u", functionId); newform = (Form_pg_proc) GETSTRUCT(newtup); /* diff --git a/contrib/sepgsql/relation.c b/contrib/sepgsql/relation.c index 59a6d9be6e..228869a520 100644 --- a/contrib/sepgsql/relation.c +++ b/contrib/sepgsql/relation.c @@ -83,7 +83,7 @@ sepgsql_attribute_post_create(Oid relOid, AttrNumber attnum) tuple = systable_getnext(sscan); if (!HeapTupleIsValid(tuple)) - elog(ERROR, "catalog lookup failed for column %d of relation %u", + elog(ERROR, "could not find tuple for column %d of relation %u", attnum, relOid); attForm = (Form_pg_attribute) GETSTRUCT(tuple); @@ -271,7 +271,7 @@ sepgsql_relation_post_create(Oid relOid) tuple = systable_getnext(sscan); if (!HeapTupleIsValid(tuple)) - elog(ERROR, "catalog lookup failed for relation %u", relOid); + elog(ERROR, "could not find tuple for relation %u", relOid); classForm = (Form_pg_class) GETSTRUCT(tuple); @@ -623,7 +623,7 @@ sepgsql_relation_setattr(Oid relOid) newtup = systable_getnext(sscan); if (!HeapTupleIsValid(newtup)) - elog(ERROR, "catalog lookup failed for relation %u", relOid); + elog(ERROR, "could not find tuple for relation %u", relOid); newform = (Form_pg_class) GETSTRUCT(newtup); /* @@ -700,7 +700,7 @@ sepgsql_relation_setattr_extra(Relation catalog, SnapshotSelf, 1, &skey); tuple = systable_getnext(sscan); if (!HeapTupleIsValid(tuple)) - elog(ERROR, "catalog lookup failed for object %u in catalog \"%s\"", + elog(ERROR, "could not find tuple for object %u in catalog \"%s\"", extra_oid, RelationGetRelationName(catalog)); datum = heap_getattr(tuple, anum_relation_id, diff --git a/contrib/sepgsql/schema.c b/contrib/sepgsql/schema.c index 940384bf40..d418577b75 100644 --- a/contrib/sepgsql/schema.c +++ b/contrib/sepgsql/schema.c @@ -67,7 +67,7 @@ sepgsql_schema_post_create(Oid namespaceId) SnapshotSelf, 1, &skey); tuple = systable_getnext(sscan); if (!HeapTupleIsValid(tuple)) - elog(ERROR, "catalog lookup failed for namespace %u", namespaceId); + elog(ERROR, "could not find tuple for namespace %u", namespaceId); nspForm = (Form_pg_namespace) GETSTRUCT(tuple); nsp_name = NameStr(nspForm->nspname); diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 387a3be701..304e3c4bc3 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -2738,7 +2738,7 @@ ExecGrant_Largeobject(InternalGrant *istmt) tuple = systable_getnext(scan); if (!HeapTupleIsValid(tuple)) - elog(ERROR, "cache lookup failed for large object %u", loid); + elog(ERROR, "could not find tuple for large object %u", loid); form_lo_meta = (Form_pg_largeobject_metadata) GETSTRUCT(tuple); @@ -5503,7 +5503,7 @@ recordExtObjInitPriv(Oid objoid, Oid classoid) tuple = systable_getnext(scan); if (!HeapTupleIsValid(tuple)) - elog(ERROR, "cache lookup failed for large object %u", objoid); + elog(ERROR, "could not find tuple for large object %u", objoid); aclDatum = heap_getattr(tuple, Anum_pg_largeobject_metadata_lomacl, diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 6bc05cab3a..be16cf66f4 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -3345,7 +3345,7 @@ getObjectDescription(const ObjectAddress *object) tuple = systable_getnext(sscan); if (!HeapTupleIsValid(tuple)) - elog(ERROR, "cache lookup failed for policy %u", + elog(ERROR, "could not find tuple for policy %u", object->objectId); form_policy = (Form_pg_policy) GETSTRUCT(tuple); diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 37fa1458be..5c5a9e11ab 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -874,7 +874,8 @@ get_partition_parent(Oid relid) NULL, 2, key); tuple = systable_getnext(scan); - Assert(HeapTupleIsValid(tuple)); + if (!HeapTupleIsValid(tuple)) + elog(ERROR, "could not find tuple for parent of relation %u", relid); form = (Form_pg_inherits) GETSTRUCT(tuple); result = form->inhparent; diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index c3718b08c1..e8126a38a9 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -2387,7 +2387,7 @@ pg_extension_config_dump(PG_FUNCTION_ARGS) extTup = systable_getnext(extScan); if (!HeapTupleIsValid(extTup)) /* should not happen */ - elog(ERROR, "extension with oid %u does not exist", + elog(ERROR, "could not find tuple for extension %u", CurrentExtensionObject); memset(repl_val, 0, sizeof(repl_val)); @@ -2535,7 +2535,7 @@ extension_config_remove(Oid extensionoid, Oid tableoid) extTup = systable_getnext(extScan); if (!HeapTupleIsValid(extTup)) /* should not happen */ - elog(ERROR, "extension with oid %u does not exist", + elog(ERROR, "could not find tuple for extension %u", extensionoid); /* Search extconfig for the tableoid */ @@ -2736,7 +2736,8 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o extTup = systable_getnext(extScan); if (!HeapTupleIsValid(extTup)) /* should not happen */ - elog(ERROR, "extension with oid %u does not exist", extensionOid); + elog(ERROR, "could not find tuple for extension %u", + extensionOid); /* Copy tuple so we can modify it below */ extTup = heap_copytuple(extTup); @@ -3057,7 +3058,7 @@ ApplyExtensionUpdates(Oid extensionOid, extTup = systable_getnext(extScan); if (!HeapTupleIsValid(extTup)) /* should not happen */ - elog(ERROR, "extension with oid %u does not exist", + elog(ERROR, "could not find tuple for extension %u", extensionOid); extForm = (Form_pg_extension) GETSTRUCT(extTup); diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index 824d7572fa..6a0d273bd2 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -1851,7 +1851,7 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, heap_close(relation, AccessShareLock); return NULL; } - elog(ERROR, "cache lookup failed for constraint %u", constraintId); + elog(ERROR, "could not find tuple for constraint %u", constraintId); } conForm = (Form_pg_constraint) GETSTRUCT(tup); -- 2.40.0