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);