Ensure that BEFORE STATEMENT triggers fire the right number of times.

Commit 0f79440 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

Author: tglsfdc

doc/src/sgml/ref/create_trigger.sgml

@@ -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>

src/backend/commands/trigger.c

@@ -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;
 }
 
 /*

src/test/regress/expected/triggers.out

@@ -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

src/test/regress/sql/triggers.sql

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