Date: Sat, 9 Feb 2019 15:56:46 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Amit Langote <amitlangote09@gmail.com>, Peter Geoghegan <pg@bowt.ie>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Fixing findDependentObjects()'s dependency on scan order
 (regressions in DROP diagnostic messages)
Message-ID: <20190209185646.GA12234@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="J2SCkAp4GZ/dPZZf"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190209174123.GA6498@alvherre.pgsql>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--J2SCkAp4GZ/dPZZf
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2019-Feb-09, Alvaro Herrera wrote:

> I'll put this in the comment.

Attached.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--J2SCkAp4GZ/dPZZf
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v2-fix-trigger-drop.patch"

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 910e5deaa3f..0d32262c4c3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8008,10 +8008,12 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
 			ReleaseSysCache(partcontup);
 
 			/*
-			 * Looks good!  Attach this constraint.  Note that the action
-			 * triggers are no longer needed, so remove them.  We identify
-			 * them because they have our constraint OID, as well as being
-			 * on the referenced rel.
+			 * Looks good!  Attach this constraint.  The action triggers are
+			 * no longer needed -- the parent table already has equivalent
+			 * ones, and those will be able to reach the partition, therefore
+			 * the ones in the partition are redundant, so remove them.  We
+			 * identify them because they have our constraint OID, as well as
+			 * being on the referenced rel.
 			 */
 			trigrel = heap_open(TriggerRelationId, RowExclusiveLock);
 			ScanKeyInit(&key,
@@ -8024,17 +8026,30 @@ CloneFkReferencing(Relation pg_constraint, Relation parentRel,
 			while ((trigtup = systable_getnext(scan)) != NULL)
 			{
 				Form_pg_trigger	trgform = (Form_pg_trigger) GETSTRUCT(trigtup);
+				ObjectAddress	trigger;
 
 				if (trgform->tgconstrrelid != fk->conrelid)
 					continue;
 				if (trgform->tgrelid != fk->confrelid)
 					continue;
 
-				deleteDependencyRecordsForClass(TriggerRelationId,
-												HeapTupleGetOid(trigtup),
-												ConstraintRelationId,
-												DEPENDENCY_INTERNAL);
-				CatalogTupleDelete(trigrel, &trigtup->t_self);
+				/*
+				 * The constraint is originally set up to contain this trigger
+				 * as an implementation object, so there's a dependency record
+				 * that links the two; however, since the trigger is no longer
+				 * needed, we remove the dependency link in order to be able
+				 * to drop the trigger while keeping the constraint intact.
+				 */
+				deleteDependencyRecordsFor(TriggerRelationId,
+										   HeapTupleGetOid(trigtup),
+										   false);
+				/* make dependency deletion visible to performDeletion */
+				CommandCounterIncrement();
+				ObjectAddressSet(trigger, TriggerRelationId,
+								 HeapTupleGetOid(trigtup));
+				performDeletion(&trigger, DROP_RESTRICT, 0);
+				/* make trigger drop visible, in case the loop iterates */
+				CommandCounterIncrement();
 			}
 
 			systable_endscan(scan);

--J2SCkAp4GZ/dPZZf--