Record parents of triggers

This let us get rid of a recently introduced ugly hack (commit
1fa846f1c9af).

Author: Álvaro Herrera
Reviewed-by: Amit Langote, Tom Lane
Discussion: https://postgr.es/m/20200217215641.GA29784@alvherre.pgsql

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index a10b66569b98fa2b7245aa70dcf2daf84c17ea26..34bc0d05266ebbffc82eb2e8364cc7b441db8744 100644 (file)
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -6951,6 +6951,17 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
       <entry>The table this trigger is on</entry>
      </row>
 
+     <row>
+      <entry><structfield>tgparentid</structfield></entry>
+      <entry><type>oid</type></entry>
+      <entry><literal><link linkend="catalog-pg-trigger"><structname>pg_trigger</structname></link>.oid</literal></entry>
+      <entry>
+       Parent trigger that this trigger is cloned from, zero if not a clone;
+       this happens when partitions are created or attached to a partitioned
+       table.
+      </entry>
+     </row>
+
      <row>
       <entry><structfield>tgname</structfield></entry>
       <entry><type>name</type></entry>

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b7c8d663fc4dbe7fba77cf9b91d3f54a25524f95..02a7c04fdb7791b4e92df852e49fae8f15b8ea40 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -16447,54 +16447,6 @@ out:
        MemoryContextDelete(cxt);
 }
 
-/*
- * isPartitionTrigger
- *             Subroutine for CloneRowTriggersToPartition: determine whether
- *             the given trigger has been cloned from another one.
- *
- * We use pg_depend as a proxy for this, since we don't have any direct
- * evidence.  This is an ugly hack to cope with a catalog deficiency.
- * Keep away from children.  Do not stare with naked eyes.  Do not propagate.
- */
-static bool
-isPartitionTrigger(Oid trigger_oid)
-{
-       Relation        pg_depend;
-       ScanKeyData key[2];
-       SysScanDesc scan;
-       HeapTuple       tup;
-       bool            found = false;
-
-       pg_depend = table_open(DependRelationId, AccessShareLock);
-
-       ScanKeyInit(&key[0], Anum_pg_depend_classid,
-                               BTEqualStrategyNumber,
-                               F_OIDEQ,
-                               ObjectIdGetDatum(TriggerRelationId));
-       ScanKeyInit(&key[1], Anum_pg_depend_objid,
-                               BTEqualStrategyNumber,
-                               F_OIDEQ,
-                               ObjectIdGetDatum(trigger_oid));
-
-       scan = systable_beginscan(pg_depend, DependDependerIndexId,
-                                                         true, NULL, 2, key);
-       while ((tup = systable_getnext(scan)) != NULL)
-       {
-               Form_pg_depend dep = (Form_pg_depend) GETSTRUCT(tup);
-
-               if (dep->refclassid == TriggerRelationId)
-               {
-                       found = true;
-                       break;
-               }
-       }
-
-       systable_endscan(scan);
-       table_close(pg_depend, AccessShareLock);
-
-       return found;
-}
-
 /*
  * CloneRowTriggersToPartition
  *             subroutine for ATExecAttachPartition/DefineRelation to create row
@@ -16537,11 +16489,10 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
 
                /*
                 * Internal triggers require careful examination.  Ideally, we don't
-                * clone them.
-                *
-                * However, if our parent is a partitioned relation, there might be
-                * internal triggers that need cloning.  In that case, we must skip
-                * clone it if the trigger on parent depends on another trigger.
+                * clone them.  However, if our parent is itself a partition, there
+                * might be internal triggers that must not be skipped; for example,
+                * triggers on our parent that are in turn clones from its parent (our
+                * grandparent) are marked internal, yet they are to be cloned.
                 *
                 * Note we dare not verify that the other trigger belongs to an
                 * ancestor relation of our parent, because that creates deadlock
@@ -16549,7 +16500,7 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
                 */
                if (trigForm->tgisinternal &&
                        (!parent->rd_rel->relispartition ||
-                        !isPartitionTrigger(trigForm->oid)))
+                        !OidIsValid(trigForm->tgparentid)))
                        continue;
 
                /*

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index b9b1262e300279a6d9f664d4a76b3930a7676d54..6e8b7223fe52abb179a398d61e83290c68e96797 100644 (file)
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -847,6 +847,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
 
        values[Anum_pg_trigger_oid - 1] = ObjectIdGetDatum(trigoid);
        values[Anum_pg_trigger_tgrelid - 1] = ObjectIdGetDatum(RelationGetRelid(rel));
+       values[Anum_pg_trigger_tgparentid - 1] = ObjectIdGetDatum(parentTriggerOid);
        values[Anum_pg_trigger_tgname - 1] = DirectFunctionCall1(namein,
                                                                                                                         CStringGetDatum(trigname));
        values[Anum_pg_trigger_tgfoid - 1] = ObjectIdGetDatum(funcoid);

diff --git a/src/include/catalog/pg_trigger.h b/src/include/catalog/pg_trigger.h
index 2f30c7c2f9442a50fc51d84ff50b4210c384a979..9612b9bdd65b0996e95617d5de47b819dc1d5445 100644 (file)
--- a/src/include/catalog/pg_trigger.h
+++ b/src/include/catalog/pg_trigger.h
@@ -35,6 +35,7 @@ CATALOG(pg_trigger,2620,TriggerRelationId)
 {
        Oid                     oid;                    /* oid */
        Oid                     tgrelid;                /* relation trigger is attached to */
+       Oid                     tgparentid;             /* OID of parent trigger, if any */
        NameData        tgname;                 /* trigger's name */
        Oid                     tgfoid;                 /* OID of function to be called */
        int16           tgtype;                 /* BEFORE/AFTER/INSTEAD, UPDATE/DELETE/INSERT,

diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out
index 45b421b747a1a406059ab1bea48933bc7241da90..192445878d21d823a02e8ef7e7a4d5f702c1cb99 100644 (file)
--- a/src/test/regress/expected/sanity_check.out
+++ b/src/test/regress/expected/sanity_check.out
@@ -208,6 +208,9 @@ timestamp_tbl|f
 timestamptz_tbl|f
 timetz_tbl|f
 tmp|f
+trigger_parted|t
+trigger_parted_p1|t
+trigger_parted_p1_1|t
 varchar_tbl|f
 view_base_table|t
 -- restore normal output mode

diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index b91fbd0648162becb1e77fbc4049762faf714dbb..22e65cc1ecedd4503e5de10bf97294efdc7009c9 100644 (file)
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2928,3 +2928,12 @@ drop table self_ref;
 drop function dump_insert();
 drop function dump_update();
 drop function dump_delete();
+-- Leave around some objects for other tests
+create table trigger_parted (a int primary key) partition by list (a);
+create function trigger_parted_trigfunc() returns trigger language plpgsql as
+  $$ begin end; $$;
+create trigger aft_row after insert or update on trigger_parted
+  for each row execute function trigger_parted_trigfunc();
+create table trigger_parted_p1 partition of trigger_parted for values in (1)
+  partition by list (a);
+create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);

diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 7cd835449c5f960437542d1143b1da44dbef049d..0f61fdf0ea2c13c9819ace3f389e774db1cbc1cf 100644 (file)
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -2213,3 +2213,13 @@ drop table self_ref;
 drop function dump_insert();
 drop function dump_update();
 drop function dump_delete();
+
+-- Leave around some objects for other tests
+create table trigger_parted (a int primary key) partition by list (a);
+create function trigger_parted_trigfunc() returns trigger language plpgsql as
+  $$ begin end; $$;
+create trigger aft_row after insert or update on trigger_parted
+  for each row execute function trigger_parted_trigfunc();
+create table trigger_parted_p1 partition of trigger_parted for values in (1)
+  partition by list (a);
+create table trigger_parted_p1_1 partition of trigger_parted_p1 for values in (1);