]> granicus.if.org Git - postgresql/commitdiff
Ensure that BEFORE STATEMENT triggers fire the right number of times.
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 17 Sep 2017 16:16:38 +0000 (12:16 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 17 Sep 2017 16:16:38 +0000 (12:16 -0400)
Commit 0f79440fb introduced mechanism to keep AFTER STATEMENT triggers
from firing more than once per statement, which was formerly possible
if more than one FK enforcement action had to be applied to a given
table.  Add a similar mechanism for BEFORE STATEMENT triggers, so that
we don't have the unexpected situation of firing BEFORE STATEMENT
triggers more often than AFTER STATEMENT.

As with the previous patch, back-patch to v10.

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

doc/src/sgml/ref/create_trigger.sgml
src/backend/commands/trigger.c
src/test/regress/expected/triggers.out
src/test/regress/sql/triggers.sql

index 065c82727109a6b1f2b8daa72a383702ef3ece8a..2496250bed8c8702e2e033220ddb1d5de131892f 100644 (file)
@@ -250,7 +250,8 @@ CREATE [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable>
       One of <literal>INSERT</literal>, <literal>UPDATE</literal>,
       <literal>DELETE</literal>, or <literal>TRUNCATE</literal>;
       this specifies the event that will fire the trigger. Multiple
-      events can be specified using <literal>OR</literal>.
+      events can be specified using <literal>OR</literal>, except when
+      transition relations are requested.
      </para>
 
      <para>
@@ -263,7 +264,10 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</
       is mentioned as a target of the <command>UPDATE</> command.
      </para>
 
-     <para><literal>INSTEAD OF UPDATE</> events do not support lists of columns.
+     <para>
+      <literal>INSTEAD OF UPDATE</> events do not allow a list of columns.
+      A column list cannot be specified when requesting transition relations,
+      either.
      </para>
     </listitem>
    </varlistentry>
@@ -490,9 +494,9 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</
    trigger that requests transition relations will cause the foreign-key
    enforcement actions triggered by a single SQL command to be split into
    multiple steps, each with its own transition relation(s).  In such cases,
-   any <literal>AFTER STATEMENT</> triggers that are present will be fired
-   once per creation of a transition relation, ensuring that the triggers see
-   each affected row once and only once.
+   any statement-level triggers that are present will be fired once per
+   creation of a transition relation set, ensuring that the triggers see
+   each affected row in a transition relation once and only once.
   </para>
 
   <para>
index 7e391a109213eec47e23a46e0fc3afe64d63b28c..7b411c130bbc679e6901618c4dda675c95490b4a 100644 (file)
@@ -100,6 +100,7 @@ static void AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
                                          List *recheckIndexes, Bitmapset *modifiedCols,
                                          TransitionCaptureState *transition_capture);
 static void AfterTriggerEnlargeQueryState(void);
+static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType);
 
 
 /*
@@ -2229,6 +2230,11 @@ ExecBSInsertTriggers(EState *estate, ResultRelInfo *relinfo)
        if (!trigdesc->trig_insert_before_statement)
                return;
 
+       /* no-op if we already fired BS triggers in this context */
+       if (before_stmt_triggers_fired(RelationGetRelid(relinfo->ri_RelationDesc),
+                                                                  CMD_INSERT))
+               return;
+
        LocTriggerData.type = T_TriggerData;
        LocTriggerData.tg_event = TRIGGER_EVENT_INSERT |
                TRIGGER_EVENT_BEFORE;
@@ -2439,6 +2445,11 @@ ExecBSDeleteTriggers(EState *estate, ResultRelInfo *relinfo)
        if (!trigdesc->trig_delete_before_statement)
                return;
 
+       /* no-op if we already fired BS triggers in this context */
+       if (before_stmt_triggers_fired(RelationGetRelid(relinfo->ri_RelationDesc),
+                                                                  CMD_DELETE))
+               return;
+
        LocTriggerData.type = T_TriggerData;
        LocTriggerData.tg_event = TRIGGER_EVENT_DELETE |
                TRIGGER_EVENT_BEFORE;
@@ -2651,6 +2662,11 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
        if (!trigdesc->trig_update_before_statement)
                return;
 
+       /* no-op if we already fired BS triggers in this context */
+       if (before_stmt_triggers_fired(RelationGetRelid(relinfo->ri_RelationDesc),
+                                                                  CMD_UPDATE))
+               return;
+
        updatedCols = GetUpdatedColumns(relinfo, estate);
 
        LocTriggerData.type = T_TriggerData;
@@ -3523,11 +3539,11 @@ typedef struct AfterTriggerEventList
  *
  * We create an AfterTriggersTableData struct for each target table of the
  * current query, and each operation mode (INSERT/UPDATE/DELETE), that has
- * either transition tables or AFTER STATEMENT triggers.  This is used to
+ * either transition tables or statement-level triggers.  This is used to
  * hold the relevant transition tables, as well as info tracking whether
- * we already queued the AFTER STATEMENT triggers.  (We use that info to
- * prevent, as much as possible, firing the same AFTER STATEMENT trigger
- * more than once per statement.)  These structs, along with the transition
+ * we already queued the statement triggers.  (We use that info to prevent
+ * firing the same statement triggers more than once per statement, or really
+ * once per transition table set.)  These structs, along with the transition
  * table tuplestores, live in the (sub)transaction's CurTransactionContext.
  * That's sufficient lifespan because we don't allow transition tables to be
  * used by deferrable triggers, so they only need to survive until
@@ -3576,8 +3592,9 @@ struct AfterTriggersTableData
        Oid                     relid;                  /* target table's OID */
        CmdType         cmdType;                /* event type, CMD_INSERT/UPDATE/DELETE */
        bool            closed;                 /* true when no longer OK to add tuples */
-       bool            stmt_trig_done; /* did we already queue stmt-level triggers? */
-       AfterTriggerEventList stmt_trig_events; /* if so, saved list pointer */
+       bool            before_trig_done;       /* did we already queue BS triggers? */
+       bool            after_trig_done;        /* did we already queue AS triggers? */
+       AfterTriggerEventList after_trig_events;        /* if so, saved list pointer */
        Tuplestorestate *old_tuplestore;        /* "old" transition table, if any */
        Tuplestorestate *new_tuplestore;        /* "new" transition table, if any */
 };
@@ -5650,6 +5667,37 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
        }
 }
 
+/*
+ * Detect whether we already queued BEFORE STATEMENT triggers for the given
+ * relation + operation, and set the flag so the next call will report "true".
+ */
+static bool
+before_stmt_triggers_fired(Oid relid, CmdType cmdType)
+{
+       bool            result;
+       AfterTriggersTableData *table;
+
+       /* Check state, like AfterTriggerSaveEvent. */
+       if (afterTriggers.query_depth < 0)
+               elog(ERROR, "before_stmt_triggers_fired() called outside of query");
+
+       /* Be sure we have enough space to record events at this query depth. */
+       if (afterTriggers.query_depth >= afterTriggers.maxquerydepth)
+               AfterTriggerEnlargeQueryState();
+
+       /*
+        * We keep this state in the AfterTriggersTableData that also holds
+        * transition tables for the relation + operation.  In this way, if we are
+        * forced to make a new set of transition tables because more tuples get
+        * entered after we've already fired triggers, we will allow a new set of
+        * statement triggers to get queued.
+        */
+       table = GetAfterTriggersTableData(relid, cmdType);
+       result = table->before_trig_done;
+       table->before_trig_done = true;
+       return result;
+}
+
 /*
  * If we previously queued a set of AFTER STATEMENT triggers for the given
  * relation + operation, and they've not been fired yet, cancel them.  The
@@ -5684,7 +5732,7 @@ cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent)
         */
        table = GetAfterTriggersTableData(relid, cmdType);
 
-       if (table->stmt_trig_done)
+       if (table->after_trig_done)
        {
                /*
                 * We want to start scanning from the tail location that existed just
@@ -5695,10 +5743,10 @@ cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent)
                AfterTriggerEvent event;
                AfterTriggerEventChunk *chunk;
 
-               if (table->stmt_trig_events.tail)
+               if (table->after_trig_events.tail)
                {
-                       chunk = table->stmt_trig_events.tail;
-                       event = (AfterTriggerEvent) table->stmt_trig_events.tailfree;
+                       chunk = table->after_trig_events.tail;
+                       event = (AfterTriggerEvent) table->after_trig_events.tailfree;
                }
                else
                {
@@ -5737,8 +5785,8 @@ cancel_prior_stmt_triggers(Oid relid, CmdType cmdType, int tgevent)
 done:
 
        /* In any case, save current insertion point for next time */
-       table->stmt_trig_done = true;
-       table->stmt_trig_events = qs->events;
+       table->after_trig_done = true;
+       table->after_trig_events = qs->events;
 }
 
 /*
index 3ab6be3421c454dae5b2d8c61fbd6b20f516606c..85d948741ed12411e98b7d8770235c69b5700da7 100644 (file)
@@ -2289,6 +2289,9 @@ create table refd_table (a int primary key, b text);
 create table trig_table (a int, b text,
   foreign key (a) references refd_table on update cascade on delete cascade
 );
+create trigger trig_table_before_trig
+  before insert or update or delete on trig_table
+  for each statement execute procedure trigger_func('trig_table');
 create trigger trig_table_insert_trig
   after insert on trig_table referencing new table as new_table
   for each statement execute procedure dump_insert();
@@ -2309,8 +2312,10 @@ insert into trig_table values
   (2, 'two b'),
   (3, 'three a'),
   (3, 'three b');
+NOTICE:  trigger_func(trig_table) called: action = INSERT, when = BEFORE, level = STATEMENT
 NOTICE:  trigger = trig_table_insert_trig, new table = (1,"one a"), (1,"one b"), (2,"two a"), (2,"two b"), (3,"three a"), (3,"three b")
 update refd_table set a = 11 where b = 'one';
+NOTICE:  trigger_func(trig_table) called: action = UPDATE, when = BEFORE, level = STATEMENT
 NOTICE:  trigger = trig_table_update_trig, old table = (1,"one a"), (1,"one b"), new table = (11,"one a"), (11,"one b")
 select * from trig_table;
  a  |    b    
@@ -2324,6 +2329,7 @@ select * from trig_table;
 (6 rows)
 
 delete from refd_table where length(b) = 3;
+NOTICE:  trigger_func(trig_table) called: action = DELETE, when = BEFORE, level = STATEMENT
 NOTICE:  trigger = trig_table_delete_trig, old table = (2,"two a"), (2,"two b"), (11,"one a"), (11,"one b")
 select * from trig_table;
  a |    b    
@@ -2338,6 +2344,9 @@ drop table refd_table, trig_table;
 --
 create table self_ref (a int primary key,
                        b int references self_ref(a) on delete cascade);
+create trigger self_ref_before_trig
+  before delete on self_ref
+  for each statement execute procedure trigger_func('self_ref');
 create trigger self_ref_r_trig
   after delete on self_ref referencing old table as old_table
   for each row execute procedure dump_delete();
@@ -2346,7 +2355,9 @@ create trigger self_ref_s_trig
   for each statement execute procedure dump_delete();
 insert into self_ref values (1, null), (2, 1), (3, 2);
 delete from self_ref where a = 1;
+NOTICE:  trigger_func(self_ref) called: action = DELETE, when = BEFORE, level = STATEMENT
 NOTICE:  trigger = self_ref_r_trig, old table = (1,), (2,1)
+NOTICE:  trigger_func(self_ref) called: action = DELETE, when = BEFORE, level = STATEMENT
 NOTICE:  trigger = self_ref_r_trig, old table = (1,), (2,1)
 NOTICE:  trigger = self_ref_s_trig, old table = (1,), (2,1)
 NOTICE:  trigger = self_ref_r_trig, old table = (3,2)
@@ -2355,6 +2366,7 @@ NOTICE:  trigger = self_ref_s_trig, old table = (3,2)
 drop trigger self_ref_r_trig on self_ref;
 insert into self_ref values (1, null), (2, 1), (3, 2), (4, 3);
 delete from self_ref where a = 1;
+NOTICE:  trigger_func(self_ref) called: action = DELETE, when = BEFORE, level = STATEMENT
 NOTICE:  trigger = self_ref_s_trig, old table = (1,), (2,1), (3,2), (4,3)
 drop table self_ref;
 -- cleanup
index 30bb7d17b088c596faad75d8a6767fc440159a03..2b2236ed7d9362696d47dfad5b90642c69afd191 100644 (file)
@@ -1795,6 +1795,9 @@ create table trig_table (a int, b text,
   foreign key (a) references refd_table on update cascade on delete cascade
 );
 
+create trigger trig_table_before_trig
+  before insert or update or delete on trig_table
+  for each statement execute procedure trigger_func('trig_table');
 create trigger trig_table_insert_trig
   after insert on trig_table referencing new table as new_table
   for each statement execute procedure dump_insert();
@@ -1834,6 +1837,9 @@ drop table refd_table, trig_table;
 create table self_ref (a int primary key,
                        b int references self_ref(a) on delete cascade);
 
+create trigger self_ref_before_trig
+  before delete on self_ref
+  for each statement execute procedure trigger_func('self_ref');
 create trigger self_ref_r_trig
   after delete on self_ref referencing old table as old_table
   for each row execute procedure dump_delete();