Fix trigger drop procedure

After commit 123cc697a8eb, we remove redundant FK action triggers during
partition ATTACH by merely deleting the catalog tuple, but that's wrong:
it should use performDeletion() instead.  Repair, and make the comments
more explicit.

Per code review from Tom Lane.

Discussion: https://postgr.es/m/18885.1549642539@sss.pgh.pa.us

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 35a9ade059025bd3da417deefa9f26bcaae35cb2..b66d194b80ddca834c170ab9ef1ce873eee43233 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7919,10 +7919,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 in
+            * the new partition become redundant -- the parent table already
+            * has equivalent ones, and those will be able to reach the
+            * partition.  Remove the ones in the partition.  We identify them
+            * because they have our constraint OID, as well as being on the
+            * referenced rel.
             */
            trigrel = heap_open(TriggerRelationId, RowExclusiveLock);
            ScanKeyInit(&key,
@@ -7935,17 +7937,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,
-                                               trgform->oid,
-                                               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,
+                                          trgform->oid,
+                                          false);
+               /* make dependency deletion visible to performDeletion */
+               CommandCounterIncrement();
+               ObjectAddressSet(trigger, TriggerRelationId,
+                                trgform->oid);
+               performDeletion(&trigger, DROP_RESTRICT, 0);
+               /* make trigger drop visible, in case the loop iterates */
+               CommandCounterIncrement();
            }
 
            systable_endscan(scan);