Add more sanity checks with callers of changeDependencyFor()

michaelpq · michaelpq · commit 9b286858e3ab · 2023-07-10T13:08:10.000+09:00
changeDependencyFor() returns the number of pg_depend entries changed, or 0 if there is a problem. The callers of this routine expect only one dependency to change, but they did not check for the result returned. The following code paths gain checks: - Namespace for extensions. - Namespace for various object types (see AlterObjectNamespace). - Planner support function for a function. Some existing error messages related to all that are reworded to be more consistent with the project style, and the new error messages added follow the same style. This change has exposed one bug fixed a bit earlier with bd5ddbe. Reviewed-by: Heikki Linnakangas, Akshat Jaimini Discussion: https://postgr.es/m/ZJzD/rn+UbloKjB7@paquier.xyz
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
@@ -847,9 +847,11 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid)
 	pfree(nulls);
 	pfree(replaces);
 
-	/* update dependencies to point to the new schema */
-	changeDependencyFor(classId, objid,
-						NamespaceRelationId, oldNspOid, nspOid);
+	/* update dependency to point to the new schema */
+	if (changeDependencyFor(classId, objid,
+							NamespaceRelationId, oldNspOid, nspOid) != 1)
+		elog(ERROR, "could not change schema dependency for object %u",
+			 objid);
 
 	InvokeObjectPostAlterHook(classId, objid, 0);
 
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
@@ -1271,15 +1271,15 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
 								AccessMethodRelationId,
 								relam1,
 								relam2) != 1)
-			elog(ERROR, "failed to change access method dependency for relation \"%s.%s\"",
+			elog(ERROR, "could not change access method dependency for relation \"%s.%s\"",
 				 get_namespace_name(get_rel_namespace(r1)),
 				 get_rel_name(r1));
 		if (changeDependencyFor(RelationRelationId,
 								r2,
 								AccessMethodRelationId,
 								relam2,
 								relam1) != 1)
-			elog(ERROR, "failed to change access method dependency for relation \"%s.%s\"",
+			elog(ERROR, "could not change access method dependency for relation \"%s.%s\"",
 				 get_namespace_name(get_rel_namespace(r2)),
 				 get_rel_name(r2));
 	}
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
@@ -2944,9 +2944,11 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o
 
 	table_close(extRel, RowExclusiveLock);
 
-	/* update dependencies to point to the new schema */
-	changeDependencyFor(ExtensionRelationId, extensionOid,
-						NamespaceRelationId, oldNspOid, nspOid);
+	/* update dependency to point to the new schema */
+	if (changeDependencyFor(ExtensionRelationId, extensionOid,
+							NamespaceRelationId, oldNspOid, nspOid) != 1)
+		elog(ERROR, "could not change schema dependency for extension %s",
+			 NameStr(extForm->extname));
 
 	InvokeObjectPostAlterHook(ExtensionRelationId, extensionOid, 0);
 
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
@@ -1450,9 +1450,13 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 
 		/* Add or replace dependency on support function */
 		if (OidIsValid(procForm->prosupport))
-			changeDependencyFor(ProcedureRelationId, funcOid,
-								ProcedureRelationId, procForm->prosupport,
-								newsupport);
+		{
+			if (changeDependencyFor(ProcedureRelationId, funcOid,
+									ProcedureRelationId, procForm->prosupport,
+									newsupport) != 1)
+				elog(ERROR, "could not change support dependency for function %s",
+					 get_func_name(funcOid));
+		}
 		else
 		{
 			ObjectAddress referenced;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
@@ -16608,7 +16608,7 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,
 								NamespaceRelationId,
 								oldNspOid,
 								newNspOid) != 1)
-			elog(ERROR, "failed to change schema dependency for relation \"%s\"",
+			elog(ERROR, "could not change schema dependency for relation \"%s\"",
 				 NameStr(classForm->relname));
 	}
 	if (!already_done)
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
@@ -4059,7 +4059,7 @@ AlterTypeNamespaceInternal(Oid typeOid, Oid nspOid,
 		!isImplicitArray)
 		if (changeDependencyFor(TypeRelationId, typeOid,
 								NamespaceRelationId, oldNspOid, nspOid) != 1)
-			elog(ERROR, "failed to change schema dependency for type %s",
+			elog(ERROR, "could not change schema dependency for type \"%s\"",
 				 format_type_be(typeOid));
 
 	InvokeObjectPostAlterHook(TypeRelationId, typeOid, 0);

Original file line number	Diff line number	Diff line change
`@@ -16608,7 +16608,7 @@ AlterRelationNamespaceInternal(Relation classRel, Oid relOid,`
`16608`	`16608`	`NamespaceRelationId,`
`16609`	`16609`	`oldNspOid,`
`16610`	`16610`	`newNspOid) != 1)`
`16611`		`- elog(ERROR, "failed to change schema dependency for relation \"%s\"",`
	`16611`	`+ elog(ERROR, "could not change schema dependency for relation \"%s\"",`
`16612`	`16612`	`NameStr(classForm->relname));`
`16613`	`16613`	`}`
`16614`	`16614`	`if (!already_done)`