From e7941a976688f0f5d13a5227ed4f3efe0718db9d Mon Sep 17 00:00:00 2001 From: Tom Lane 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 ", 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);