public inbox for [email protected]  

help / color / mirror / Atom feed

From: Michael Paquier <[email protected]>
To: Postgres hackers <[email protected]>
Subject: Add more sanity checks around callers of changeDependencyFor()
Date: Thu, 29 Jun 2023 08:36:30 +0900
Message-ID: <ZJzD/[email protected]> (raw)

Hi all,

While working on a different patch, I have noted three code paths that
call changeDependencyFor() but don't check that they do not return
errors.  In all the three cases (support function, extension/schema
and object/schema), it seems to me that only one dependency update is
expected.

I am adding that to the next CF.  Thoughts or comments about the
attached?
--
Michael


Attachments:

  [text/x-diff] 0001-Add-checks-for-values-returned-by-changeDependencyFo.patch (2.6K, ../ZJzD%[email protected]/2-0001-Add-checks-for-values-returned-by-changeDependencyFo.patch)
  download | inline diff:
From 2b66afedbfd27a80beae3a48e0eb362e21ef4547 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Thu, 29 Jun 2023 08:35:23 +0900
Subject: [PATCH] Add checks for values returned by changeDependencyFor()

---
 src/backend/commands/alter.c        |  6 ++++--
 src/backend/commands/extension.c    |  6 ++++--
 src/backend/commands/functioncmds.c | 10 +++++++---
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index e95dc31bde..455d8dd86a 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -848,8 +848,10 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid)
 	pfree(replaces);
 
 	/* update dependencies to point to the new schema */
-	changeDependencyFor(classId, objid,
-						NamespaceRelationId, oldNspOid, nspOid);
+	if (changeDependencyFor(classId, objid,
+						   NamespaceRelationId, oldNspOid, nspOid) != 1)
+		elog(ERROR, "failed to change schema dependency for object %u",
+			 objid);
 
 	InvokeObjectPostAlterHook(classId, objid, 0);
 
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 0eabe18335..e95642e14f 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -2948,8 +2948,10 @@ 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);
+	if (changeDependencyFor(ExtensionRelationId, extensionOid,
+							NamespaceRelationId, oldNspOid, nspOid) != 1)
+		elog(ERROR, "failed to 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
index 49c7864c7c..5ec2c002d2 100644
--- 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, "failed to change support dependency for function %s",
+					 get_func_name(funcOid));
+		}
 		else
 		{
 			ObjectAddress referenced;
-- 
2.40.1



  [application/pgp-signature] signature.asc (833B, ../ZJzD%[email protected]/3-signature.asc)
  download

view thread (3+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Add more sanity checks around callers of changeDependencyFor()
  In-Reply-To: <ZJzD/[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox