Add ALTER .. NO DEPENDS ON

Commit f2fcad27d59c (9.6 era) added the ability to mark objects as
dependent an extension, but forgot to add a way for such dependencies to
be removed.  This commit fixes that oversight.

Strictly speaking this should be backpatched to 9.6, but due to lack of
demand we're not doing so at this time.

Discussion: https://postgr.es/m/20200217225333.GA30974@alvherre.pgsql
Reviewed-by: ahsan hadi <ahsan.hadi@gmail.com>
Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>

9 files changed, 146 insertions, 21 deletions

diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 596dafe19c8..fa38ee94777 100644
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -279,6 +279,55 @@ deleteDependencyRecordsForClass(Oid classId, Oid objectId,
 }
 
 /*
+ * deleteDependencyRecordsForSpecific -- delete all records with given depender
+ * classId/objectId, dependee classId/objectId, of the given deptype.
+ * Returns the number of records deleted.
+ */
+long
+deleteDependencyRecordsForSpecific(Oid classId, Oid objectId, char deptype,
+								   Oid refclassId, Oid refobjectId)
+{
+	long		count = 0;
+	Relation	depRel;
+	ScanKeyData key[2];
+	SysScanDesc scan;
+	HeapTuple	tup;
+
+	depRel = table_open(DependRelationId, RowExclusiveLock);
+
+	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 == refclassId &&
+			depform->refobjid == refobjectId &&
+			depform->deptype == deptype)
+		{
+			CatalogTupleDelete(depRel, &tup->t_self);
+			count++;
+		}
+	}
+
+	systable_endscan(scan);
+
+	table_close(depRel, RowExclusiveLock);
+
+	return count;
+}
+
+/*
  * Adjust dependency record(s) to point to a different object of the same type
  *
  * classId/objectId specify the referencing object.
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 11db9bfe922..951690b2b8d 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -421,7 +421,7 @@ ExecRenameStmt(RenameStmt *stmt)
 }
 
 /*
- * Executes an ALTER OBJECT / DEPENDS ON [EXTENSION] statement.
+ * Executes an ALTER OBJECT / [NO] DEPENDS ON EXTENSION statement.
  *
  * Return value is the address of the altered object.  refAddress is an output
  * argument which, if not null, receives the address of the object that the
@@ -433,7 +433,6 @@ ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt, ObjectAddress *refAddre
 	ObjectAddress address;
 	ObjectAddress refAddr;
 	Relation	rel;
-	List   *currexts;
 
 	address =
 		get_object_address_rv(stmt->objectType, stmt->relation, (List *) stmt->object,
@@ -463,11 +462,22 @@ ExecAlterObjectDependsStmt(AlterObjectDependsStmt *stmt, ObjectAddress *refAddre
 	if (refAddress)
 		*refAddress = refAddr;
 
-	/* Avoid duplicates */
-	currexts = getAutoExtensionsOfObject(address.classId,
-										 address.objectId);
-	if (!list_member_oid(currexts, refAddr.objectId))
-		recordDependencyOn(&address, &refAddr, DEPENDENCY_AUTO_EXTENSION);
+	if (stmt->remove)
+	{
+		deleteDependencyRecordsForSpecific(address.classId, address.objectId,
+										   DEPENDENCY_AUTO_EXTENSION,
+										   refAddr.classId, refAddr.objectId);
+	}
+	else
+	{
+		List   *currexts;
+
+		/* 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/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 1525c0de725..491452ae2d4 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3638,6 +3638,7 @@ _copyAlterObjectDependsStmt(const AlterObjectDependsStmt *from)
 	COPY_NODE_FIELD(relation);
 	COPY_NODE_FIELD(object);
 	COPY_NODE_FIELD(extname);
+	COPY_SCALAR_FIELD(remove);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 4f34189ab5c..8408c28ec69 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1449,6 +1449,7 @@ _equalAlterObjectDependsStmt(const AlterObjectDependsStmt *a, const AlterObjectD
 	COMPARE_NODE_FIELD(relation);
 	COMPARE_NODE_FIELD(object);
 	COMPARE_NODE_FIELD(extname);
+	COMPARE_SCALAR_FIELD(remove);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1219ac8c264..3c78f2d1b51 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -320,7 +320,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %type <list>	vac_analyze_option_list
 %type <node>	vac_analyze_option_arg
 %type <defelt>	drop_option
-%type <boolean>	opt_or_replace
+%type <boolean>	opt_or_replace opt_no
 				opt_grant_grant_option opt_grant_admin_option
 				opt_nowait opt_if_exists opt_with_data
 				opt_transaction_chain
@@ -9053,57 +9053,67 @@ opt_set_data: SET DATA_P							{ $$ = 1; }
  *****************************************************************************/
 
 AlterObjectDependsStmt:
-			ALTER FUNCTION function_with_argtypes DEPENDS ON EXTENSION name
+			ALTER FUNCTION function_with_argtypes opt_no DEPENDS ON EXTENSION name
 				{
 					AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
 					n->objectType = OBJECT_FUNCTION;
 					n->object = (Node *) $3;
-					n->extname = makeString($7);
+					n->extname = makeString($8);
+					n->remove = $4;
 					$$ = (Node *)n;
 				}
-			| ALTER PROCEDURE function_with_argtypes DEPENDS ON EXTENSION name
+			| ALTER PROCEDURE function_with_argtypes opt_no DEPENDS ON EXTENSION name
 				{
 					AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
 					n->objectType = OBJECT_PROCEDURE;
 					n->object = (Node *) $3;
-					n->extname = makeString($7);
+					n->extname = makeString($8);
+					n->remove = $4;
 					$$ = (Node *)n;
 				}
-			| ALTER ROUTINE function_with_argtypes DEPENDS ON EXTENSION name
+			| ALTER ROUTINE function_with_argtypes opt_no DEPENDS ON EXTENSION name
 				{
 					AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
 					n->objectType = OBJECT_ROUTINE;
 					n->object = (Node *) $3;
-					n->extname = makeString($7);
+					n->extname = makeString($8);
+					n->remove = $4;
 					$$ = (Node *)n;
 				}
-			| ALTER TRIGGER name ON qualified_name DEPENDS ON EXTENSION name
+			| ALTER TRIGGER name ON qualified_name opt_no DEPENDS ON EXTENSION name
 				{
 					AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
 					n->objectType = OBJECT_TRIGGER;
 					n->relation = $5;
 					n->object = (Node *) list_make1(makeString($3));
-					n->extname = makeString($9);
+					n->extname = makeString($10);
+					n->remove = $6;
 					$$ = (Node *)n;
 				}
-			| ALTER MATERIALIZED VIEW qualified_name DEPENDS ON EXTENSION name
+			| ALTER MATERIALIZED VIEW qualified_name opt_no DEPENDS ON EXTENSION name
 				{
 					AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
 					n->objectType = OBJECT_MATVIEW;
 					n->relation = $4;
-					n->extname = makeString($8);
+					n->extname = makeString($9);
+					n->remove = $5;
 					$$ = (Node *)n;
 				}
-			| ALTER INDEX qualified_name DEPENDS ON EXTENSION name
+			| ALTER INDEX qualified_name opt_no DEPENDS ON EXTENSION name
 				{
 					AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt);
 					n->objectType = OBJECT_INDEX;
 					n->relation = $3;
-					n->extname = makeString($7);
+					n->extname = makeString($8);
+					n->remove = $4;
 					$$ = (Node *)n;
 				}
 		;
 
+opt_no:		NO				{ $$ = true; }
+			| /* EMPTY */	{ $$ = false;	}
+		;
+
 /*****************************************************************************
  *
  * ALTER THING name SET SCHEMA name
diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h
index ab5e92bdc6f..2c6abe26a5a 100644
--- a/src/include/catalog/dependency.h
+++ b/src/include/catalog/dependency.h
@@ -196,6 +196,10 @@ extern long deleteDependencyRecordsFor(Oid classId, Oid objectId,
 extern long deleteDependencyRecordsForClass(Oid classId, Oid objectId,
 											Oid refclassId, char deptype);
 
+extern long deleteDependencyRecordsForSpecific(Oid classId, Oid objectId,
+											   char deptype,
+											   Oid refclassId, Oid refobjectId);
+
 extern long changeDependencyFor(Oid classId, Oid objectId,
 								Oid refClassId, Oid oldRefObjectId,
 								Oid newRefObjectId);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 518abe42c10..5e1ffafb91b 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2936,6 +2936,7 @@ typedef struct AlterObjectDependsStmt
 	RangeVar   *relation;		/* in case a table is involved */
 	Node	   *object;			/* name of the object */
 	Value	   *extname;		/* extension name */
+	bool		remove;			/* set true to remove dep rather than add */
 } AlterObjectDependsStmt;
 
 /* ----------------------
diff --git a/src/test/modules/test_extensions/expected/test_extdepend.out b/src/test/modules/test_extensions/expected/test_extdepend.out
index 40533e90de3..0b62015d18c 100644
--- a/src/test/modules/test_extensions/expected/test_extdepend.out
+++ b/src/test/modules/test_extensions/expected/test_extdepend.out
@@ -150,5 +150,39 @@ SELECT deptype, i.*
 (0 rows)
 
 DROP TABLE a;
+RESET search_path;
 DROP SCHEMA test_ext CASCADE;
 NOTICE:  drop cascades to extension test_ext5
+-- Fourth test: we can mark the objects as dependent, then unmark; then the
+-- drop of the extension does nothing
+SELECT * FROM test_extdep_commands \gexec
+ CREATE SCHEMA test_ext
+ CREATE EXTENSION test_ext5 SCHEMA test_ext
+ SET search_path TO test_ext
+ CREATE TABLE a (a1 int)
+
+ CREATE FUNCTION b() RETURNS TRIGGER LANGUAGE plpgsql AS
+   $$ BEGIN NEW.a1 := NEW.a1 + 42; RETURN NEW; END; $$
+ ALTER FUNCTION b() DEPENDS ON EXTENSION test_ext5
+
+ CREATE TRIGGER c BEFORE INSERT ON a FOR EACH ROW EXECUTE PROCEDURE b()
+ ALTER TRIGGER c ON a DEPENDS ON EXTENSION test_ext5
+
+ CREATE MATERIALIZED VIEW d AS SELECT * FROM a
+ ALTER MATERIALIZED VIEW d DEPENDS ON EXTENSION test_ext5
+
+ CREATE INDEX e ON a (a1)
+ ALTER INDEX e DEPENDS ON EXTENSION test_ext5
+ RESET search_path
+SET search_path TO test_ext;
+ALTER FUNCTION b() NO DEPENDS ON EXTENSION test_ext5;
+ALTER TRIGGER c ON a NO DEPENDS ON EXTENSION test_ext5;
+ALTER MATERIALIZED VIEW d NO DEPENDS ON EXTENSION test_ext5;
+ALTER INDEX e NO DEPENDS ON EXTENSION test_ext5;
+DROP EXTENSION test_ext5;
+DROP TRIGGER c ON a;
+DROP FUNCTION b();
+DROP MATERIALIZED VIEW d;
+DROP INDEX e;
+DROP SCHEMA test_ext CASCADE;
+NOTICE:  drop cascades to table a
diff --git a/src/test/modules/test_extensions/sql/test_extdepend.sql b/src/test/modules/test_extensions/sql/test_extdepend.sql
index cc170ab7097..63240a1af5d 100644
--- a/src/test/modules/test_extensions/sql/test_extdepend.sql
+++ b/src/test/modules/test_extensions/sql/test_extdepend.sql
@@ -70,6 +70,21 @@ SELECT deptype, i.*
         refobjid=(SELECT oid FROM pg_extension WHERE extname='test_ext5'))
 	OR (refclassid='pg_class'::regclass AND refobjid='test_ext.a'::regclass)
    AND NOT deptype IN ('i', 'a');
-
 DROP TABLE a;
+RESET search_path;
+DROP SCHEMA test_ext CASCADE;
+
+-- Fourth test: we can mark the objects as dependent, then unmark; then the
+-- drop of the extension does nothing
+SELECT * FROM test_extdep_commands \gexec
+SET search_path TO test_ext;
+ALTER FUNCTION b() NO DEPENDS ON EXTENSION test_ext5;
+ALTER TRIGGER c ON a NO DEPENDS ON EXTENSION test_ext5;
+ALTER MATERIALIZED VIEW d NO DEPENDS ON EXTENSION test_ext5;
+ALTER INDEX e NO DEPENDS ON EXTENSION test_ext5;
+DROP EXTENSION test_ext5;
+DROP TRIGGER c ON a;
+DROP FUNCTION b();
+DROP MATERIALIZED VIEW d;
+DROP INDEX e;
 DROP SCHEMA test_ext CASCADE;