Avoid duplicates in ALTER ... DEPENDS ON EXTENSION

If the command is attempted for an extension that the object already
depends on, silently do nothing.

In particular, this means that if a database containing multiple such
entries is dumped, the restore will silently do the right thing and
record just the first one.  (At least, in a world where pg_dump does
dump such entries -- which it doesn't currently, but it will.)

Backpatch to 9.6, where this kind of dependency was introduced.

Reviewed-by: Ibrar Ahmed, Tom Lane (offlist)
Discussion: https://postgr.es/m/20200217225333.GA30974@alvherre.pgsql

5 files changed, 55 insertions, 1 deletions

diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 6d935b4aa88..0fbac49083d 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -489,6 +489,49 @@ getExtensionOfObject(Oid classId, Oid objectId)
 }
 
 /*
+ * Return (possibly NIL) list of extensions that the given object depends on
+ * in DEPENDENCY_AUTO_EXTENSION mode.
+ */
+List *
+getAutoExtensionsOfObject(Oid classId, Oid objectId)
+{
+	List	   *result = NIL;
+	Relation	depRel;
+	ScanKeyData	key[2];
+	SysScanDesc	scan;
+	HeapTuple	tup;
+
+	depRel = heap_open(DependRelationId, AccessShareLock);
+
+	ScanKeyInit(&key[0],
+				Anum_pg_depend_classid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(classId));
+	ScanKeyInit(&key[1],
+				Anum_pg_depend_objid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(objectId));
+
+	scan = systable_beginscan(depRel, DependDependerIndexId, true,
+							  NULL, 2, key);
+
+	while (HeapTupleIsValid((tup = systable_getnext(scan))))
+	{
+		Form_pg_depend depform = (Form_pg_depend) GETSTRUCT(tup);
+
+		if (depform->refclassid == ExtensionRelationId &&
+			depform->deptype == DEPENDENCY_AUTO_EXTENSION)
+			result = lappend_oid(result, depform->refobjid);
+	}
+
+	systable_endscan(scan);
+
+	heap_close(depRel, AccessShareLock);
+
+	return result;
+}
+
+/*
  * Detect whether a sequence is marked as "owned" by a column
  *
  * An ownership marker is an AUTO dependency from the sequence to the
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 3ef3bd4b067..b12f8a60887 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -403,6 +403,7 @@ ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt, ObjectAddress *refAddre
 	ObjectAddress address;
 	ObjectAddress refAddr;
 	Relation	rel;
+	List   *currexts;
 
 	address =
 		get_object_address_rv(stmt->objectType, stmt->relation, stmt->objname,
@@ -434,7 +435,11 @@ ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt, ObjectAddress *refAddre
 	if (refAddress)
 		*refAddress = refAddr;
 
-	recordDependencyOn(&address, &refAddr, DEPENDENCY_AUTO_EXTENSION);
+	/* Avoid duplicates */
+	currexts = getAutoExtensionsOfObject(address.classId,
+										 address.objectId);
+	if (!list_member_oid(currexts, refAddr.objectId))
+		recordDependencyOn(&address, &refAddr, DEPENDENCY_AUTO_EXTENSION);
 
 	return address;
 }
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index 09b36c5c785..16f6719a2fe 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -232,6 +232,8 @@ extern long changeDependencyFor(Oid classId, Oid objectId,
 
 extern Oid	getExtensionOfObject(Oid classId, Oid objectId);
 
+extern List *getAutoExtensionsOfObject(Oid classId, Oid objectId);
+
 extern bool sequenceIsOwned(Oid seqId, Oid *tableId, int32 *colId);
 
 extern void markSequenceUnowned(Oid seqId);
diff --git a/src/test/modules/test_extensions/expected/test_extdepend.out b/src/test/modules/test_extensions/expected/test_extdepend.out
index 11e441ddd37..40533e90de3 100644
--- a/src/test/modules/test_extensions/expected/test_extdepend.out
+++ b/src/test/modules/test_extensions/expected/test_extdepend.out
@@ -47,6 +47,8 @@ SELECT * FROM test_extdep_commands \gexec
  CREATE INDEX e ON a (a1)
  ALTER INDEX e DEPENDS ON EXTENSION test_ext5
  RESET search_path
+-- A dependent object made dependent again has no effect
+ALTER FUNCTION test_ext.b() DEPENDS ON EXTENSION test_ext5;
 -- make sure we have the right dependencies on the extension
 SELECT deptype, p.*
   FROM pg_depend, pg_identify_object(classid, objid, objsubid) AS p
diff --git a/src/test/modules/test_extensions/sql/test_extdepend.sql b/src/test/modules/test_extensions/sql/test_extdepend.sql
index cf44145dcb3..cc170ab7097 100644
--- a/src/test/modules/test_extensions/sql/test_extdepend.sql
+++ b/src/test/modules/test_extensions/sql/test_extdepend.sql
@@ -27,6 +27,8 @@ COPY test_extdep_commands FROM stdin;
 SELECT * FROM test_extdep_commands;
 -- First, test that dependent objects go away when the extension is dropped.
 SELECT * FROM test_extdep_commands \gexec
+-- A dependent object made dependent again has no effect
+ALTER FUNCTION test_ext.b() DEPENDS ON EXTENSION test_ext5;
 -- make sure we have the right dependencies on the extension
 SELECT deptype, p.*
   FROM pg_depend, pg_identify_object(classid, objid, objsubid) AS p