Fire non-deferred AFTER triggers immediately upon query completion,

rather than when returning to the idle loop.  This makes no particular
difference for interactively-issued queries, but it makes a big difference
for queries issued within functions: trigger execution now occurs before
the calling function is allowed to proceed.  This responds to numerous
complaints about nonintuitive behavior of foreign key checking, such as
http://archives.postgresql.org/pgsql-bugs/2004-09/msg00020.php, and
appears to be required by the SQL99 spec.
Also take the opportunity to simplify the data structures used for the
pending-trigger list, rename them for more clarity, and squeeze out a
bit of space.

src/backend/access/transam/xact.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.186 2004/09/06 17:56:04 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.187 2004/09/10 18:39:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -138,7 +138,6 @@ static void CleanupSubTransaction(void);
 static void StartAbortedSubTransaction(void);
 static void PushTransaction(void);
 static void PopTransaction(void);
-static void CommitTransactionToLevel(int level);
 static char *CleanupAbortedSubTransactions(bool returnName);
 
 static void AtSubAbort_Memory(void);
@@ -1219,7 +1218,7 @@ StartTransaction(void)
 	 */
 	AtStart_Inval();
 	AtStart_Cache();
-	DeferredTriggerBeginXact();
+	AfterTriggerBeginXact();
 
 	/*
 	 * done with start processing, set current transaction state to "in
@@ -1253,7 +1252,7 @@ CommitTransaction(void)
 	 * committed. He'll invoke all trigger deferred until XACT before we
 	 * really start on committing the transaction.
 	 */
-	DeferredTriggerEndXact();
+	AfterTriggerEndXact();
 
 	/*
 	 * Similarly, let ON COMMIT management do its thing before we start to
@@ -1454,7 +1453,7 @@ AbortTransaction(void)
 	/*
 	 * do abort processing
 	 */
-	DeferredTriggerAbortXact();
+	AfterTriggerAbortXact();
 	AtAbort_Portals();
 	AtEOXact_LargeObject(false);	/* 'false' means it's abort */
 	AtAbort_Notify();
@@ -1672,12 +1671,6 @@ CommitTransactionCommand(void)
 			 * default state.
 			 */
 		case TBLOCK_END:
-			/* commit all open subtransactions */
-			if (s->nestingLevel > 1)
-				CommitTransactionToLevel(2);
-			s = CurrentTransactionState;
-			Assert(s->parent == NULL);
-			/* and now the outer transaction */
 			CommitTransaction();
 			s->blockState = TBLOCK_DEFAULT;
 			break;
@@ -1732,11 +1725,10 @@ CommitTransactionCommand(void)
 			break;
 
 			/*
-			 * We were issued a RELEASE command, so we end the current
-			 * subtransaction and return to the parent transaction.
-			 *
-			 * Since RELEASE can exit multiple levels of subtransaction, we
-			 * must loop here until we get out of all SUBEND'ed levels.
+			 * We were issued a COMMIT or RELEASE command, so we end the
+			 * current subtransaction and return to the parent transaction.
+			 * Lather, rinse, and repeat until we get out of all SUBEND'ed
+			 * subtransaction levels.
 			 */
 		case TBLOCK_SUBEND:
 			do
@@ -1745,6 +1737,13 @@ CommitTransactionCommand(void)
 				PopTransaction();
 				s = CurrentTransactionState;	/* changed by pop */
 			} while (s->blockState == TBLOCK_SUBEND);
+			/* If we had a COMMIT command, finish off the main xact too */
+			if (s->blockState == TBLOCK_END)
+			{
+				Assert(s->parent == NULL);
+				CommitTransaction();
+				s->blockState = TBLOCK_DEFAULT;
+			}
 			break;
 
 			/*
@@ -2238,7 +2237,6 @@ EndTransactionBlock(void)
 			 * the default state.
 			 */
 		case TBLOCK_INPROGRESS:
-		case TBLOCK_SUBINPROGRESS:
 			s->blockState = TBLOCK_END;
 			result = true;
 			break;
@@ -2254,6 +2252,22 @@ EndTransactionBlock(void)
 			s->blockState = TBLOCK_ENDABORT;
 			break;
 
+			/*
+			 * We are in a live subtransaction block.  Set up to subcommit
+			 * all open subtransactions and then commit the main transaction.
+			 */
+		case TBLOCK_SUBINPROGRESS:
+			while (s->parent != NULL)
+			{
+				Assert(s->blockState == TBLOCK_SUBINPROGRESS);
+				s->blockState = TBLOCK_SUBEND;
+				s = s->parent;
+			}
+			Assert(s->blockState == TBLOCK_INPROGRESS);
+			s->blockState = TBLOCK_END;
+			result = true;
+			break;
+
 			/*
 			 * Here we are inside an aborted subtransaction.  Go to the
 			 * "abort the whole tree" state so that
@@ -2699,8 +2713,12 @@ ReleaseCurrentSubTransaction(void)
 	if (s->blockState != TBLOCK_SUBINPROGRESS)
 		elog(ERROR, "ReleaseCurrentSubTransaction: unexpected state %s",
 			 BlockStateAsString(s->blockState));
+	Assert(s->state == TRANS_INPROGRESS);
 	MemoryContextSwitchTo(CurTransactionContext);
-	CommitTransactionToLevel(GetCurrentTransactionNestLevel());
+	CommitSubTransaction();
+	PopTransaction();
+	s = CurrentTransactionState; /* changed by pop */
+	Assert(s->state == TRANS_INPROGRESS);
 }
 
 /*
@@ -2827,28 +2845,6 @@ AbortOutOfAnyTransaction(void)
 	Assert(s->parent == NULL);
 }
 
-/*
- * CommitTransactionToLevel
- *
- * Commit everything from the current transaction level
- * up to the specified level (inclusive).
- */
-static void
-CommitTransactionToLevel(int level)
-{
-	TransactionState s = CurrentTransactionState;
-
-	Assert(s->state == TRANS_INPROGRESS);
-
-	while (s->nestingLevel >= level)
-	{
-		CommitSubTransaction();
-		PopTransaction();
-		s = CurrentTransactionState;	/* changed by pop */
-		Assert(s->state == TRANS_INPROGRESS);
-	}
-}
-
 /*
  * IsTransactionBlock --- are we within a transaction block?
  */
@@ -2975,7 +2971,7 @@ StartSubTransaction(void)
 	 */
 	AtSubStart_Inval();
 	AtSubStart_Notify();
-	DeferredTriggerBeginSubXact();
+	AfterTriggerBeginSubXact();
 
 	s->state = TRANS_INPROGRESS;
 
@@ -3011,7 +3007,7 @@ CommitSubTransaction(void)
 	AtSubCommit_childXids();
 
 	/* Post-commit cleanup */
-	DeferredTriggerEndSubXact(true);
+	AfterTriggerEndSubXact(true);
 	AtSubCommit_Portals(s->parent->transactionIdData,
 						s->parent->curTransactionOwner);
 	AtEOSubXact_LargeObject(true, s->transactionIdData,
@@ -3101,7 +3097,7 @@ AbortSubTransaction(void)
 	 */
 	AtSubAbort_Memory();
 
-	DeferredTriggerEndSubXact(false);
+	AfterTriggerEndSubXact(false);
 	AtSubAbort_Portals(s->parent->transactionIdData,
 					   s->parent->curTransactionOwner);
 	AtEOSubXact_LargeObject(false, s->transactionIdData,

src/backend/commands/copy.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.230 2004/08/29 05:06:41 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.231 2004/09/10 18:39:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1610,6 +1610,11 @@ CopyFrom(Relation rel, List *attnumlist, bool binary, bool oids,
 		}
 	}
 
+	/*
+	 * Prepare to catch AFTER triggers.
+	 */
+	AfterTriggerBeginQuery();
+
 	/*
 	 * Check BEFORE STATEMENT insertion triggers. It's debateable whether
 	 * we should do this for COPY, since it's not really an "INSERT"
@@ -1974,6 +1979,11 @@ CopyFrom(Relation rel, List *attnumlist, bool binary, bool oids,
 	 */
 	ExecASInsertTriggers(estate, resultRelInfo);
 
+	/*
+	 * Handle queued AFTER triggers
+	 */
+	AfterTriggerEndQuery();
+
 	pfree(values);
 	pfree(nulls);
 

src/backend/commands/explain.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994-5, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/explain.c,v 1.124 2004/08/29 05:06:41 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/explain.c,v 1.125 2004/09/10 18:39:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -18,6 +18,7 @@
 #include "catalog/pg_type.h"
 #include "commands/explain.h"
 #include "commands/prepare.h"
+#include "commands/trigger.h"
 #include "executor/executor.h"
 #include "executor/instrument.h"
 #include "lib/stringinfo.h"
@@ -206,6 +207,10 @@ ExplainOnePlan(QueryDesc *queryDesc, ExplainStmt *stmt,
 
 	gettimeofday(&starttime, NULL);
 
+	/* If analyzing, we need to cope with queued triggers */
+	if (stmt->analyze)
+		AfterTriggerBeginQuery();
+
 	/* call ExecutorStart to prepare the plan for execution */
 	ExecutorStart(queryDesc, false, !stmt->analyze);
 
@@ -255,12 +260,16 @@ ExplainOnePlan(QueryDesc *queryDesc, ExplainStmt *stmt,
 	}
 
 	/*
-	 * Close down the query and free resources.  Include time for this in
-	 * the total runtime.
+	 * Close down the query and free resources; also run any queued
+	 * AFTER triggers.  Include time for this in the total runtime.
 	 */
 	gettimeofday(&starttime, NULL);
 
 	ExecutorEnd(queryDesc);
+
+	if (stmt->analyze)
+		AfterTriggerEndQuery();
+
 	FreeQueryDesc(queryDesc);
 
 	CommandCounterIncrement();

src/backend/commands/portalcmds.c

@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.33 2004/08/29 05:06:41 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.34 2004/09/10 18:39:56 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -273,6 +273,7 @@ PortalCleanup(Portal portal)
 			{
 				CurrentResourceOwner = portal->resowner;
 				ExecutorEnd(queryDesc);
+				/* we do not need AfterTriggerEndQuery() here */
 			}
 			PG_CATCH();
 			{
@@ -373,6 +374,7 @@ PersistHoldablePortal(Portal portal)
 		 */
 		portal->queryDesc = NULL;		/* prevent double shutdown */
 		ExecutorEnd(queryDesc);
+		/* we do not need AfterTriggerEndQuery() here */
 
 		/*
 		 * Reset the position in the result set: ideally, this could be

src/backend/executor/functions.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.87 2004/09/06 18:10:38 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/functions.c,v 1.88 2004/09/10 18:39:57 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -17,6 +17,7 @@
 #include "access/heapam.h"
 #include "catalog/pg_proc.h"
 #include "catalog/pg_type.h"
+#include "commands/trigger.h"
 #include "executor/execdefs.h"
 #include "executor/executor.h"
 #include "executor/functions.h"
@@ -273,7 +274,10 @@ postquel_start(execution_state *es, SQLFunctionCachePtr fcache)
 
 	/* Utility commands don't need Executor. */
 	if (es->qd->operation != CMD_UTILITY)
+	{
+		AfterTriggerBeginQuery();
 		ExecutorStart(es->qd, false, false);
+	}
 
 	es->status = F_EXEC_RUN;
 }
@@ -316,7 +320,10 @@ postquel_end(execution_state *es)
 
 	/* Utility commands don't need Executor. */
 	if (es->qd->operation != CMD_UTILITY)
+	{
 		ExecutorEnd(es->qd);
+		AfterTriggerEndQuery();
+	}
 
 	FreeQueryDesc(es->qd);
 	es->qd = NULL;

src/backend/executor/spi.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.125 2004/08/29 05:06:42 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.126 2004/09/10 18:39:57 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -16,6 +16,7 @@
 
 #include "access/printtup.h"
 #include "catalog/heap.h"
+#include "commands/trigger.h"
 #include "executor/spi_priv.h"
 #include "tcop/tcopprot.h"
 #include "utils/lsyscache.h"
@@ -1434,6 +1435,8 @@ _SPI_pquery(QueryDesc *queryDesc, bool runit,
 		ResetUsage();
 #endif
 
+	AfterTriggerBeginQuery();
+
 	ExecutorStart(queryDesc, useCurrentSnapshot, false);
 
 	ExecutorRun(queryDesc, ForwardScanDirection, (long) tcount);
@@ -1447,6 +1450,11 @@ _SPI_pquery(QueryDesc *queryDesc, bool runit,
 			elog(ERROR, "consistency check on SPI tuple count failed");
 	}
 
+	ExecutorEnd(queryDesc);
+
+	/* Take care of any queued AFTER triggers */
+	AfterTriggerEndQuery();
+
 	if (queryDesc->dest->mydest == SPI)
 	{
 		SPI_processed = _SPI_current->processed;
@@ -1459,8 +1467,6 @@ _SPI_pquery(QueryDesc *queryDesc, bool runit,
 		res = SPI_OK_UTILITY;
 	}
 
-	ExecutorEnd(queryDesc);
-
 	FreeQueryDesc(queryDesc);
 
 #ifdef SPI_EXECUTOR_STATS

src/backend/tcop/postgres.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.430 2004/08/29 05:06:49 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.431 2004/09/10 18:39:59 tgl Exp $
  *
  * NOTES
  *	  this is the "main" module of the postgres backend and
@@ -1823,9 +1823,6 @@ finish_xact_command(void)
 {
 	if (xact_started)
 	{
-		/* Invoke IMMEDIATE constraint triggers */
-		DeferredTriggerEndQuery();
-
 		/* Cancel any active statement timeout before committing */
 		disable_sig_alarm(true);
 

src/backend/tcop/pquery.c

@@ -8,13 +8,14 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.85 2004/08/29 05:06:49 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/pquery.c,v 1.86 2004/09/10 18:40:00 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 
 #include "postgres.h"
 
+#include "commands/trigger.h"
 #include "executor/executor.h"
 #include "miscadmin.h"
 #include "tcop/tcopprot.h"
@@ -136,6 +137,11 @@ ProcessQuery(Query *parsetree,
 	 */
 	queryDesc = CreateQueryDesc(parsetree, plan, dest, params, false);
 
+	/*
+	 * Set up to collect AFTER triggers
+	 */
+	AfterTriggerBeginQuery();
+
 	/*
 	 * Call ExecStart to prepare the plan for execution
 	 */
@@ -185,6 +191,9 @@ ProcessQuery(Query *parsetree,
 	 */
 	ExecutorEnd(queryDesc);
 
+	/* And take care of any queued AFTER triggers */
+	AfterTriggerEndQuery();
+
 	FreeQueryDesc(queryDesc);
 }
 
@@ -290,6 +299,13 @@ PortalStart(Portal portal, ParamListInfo params)
 											params,
 											false);
 
+				/*
+				 * We do *not* call AfterTriggerBeginQuery() here.  We
+				 * assume that a SELECT cannot queue any triggers.  It
+				 * would be messy to support triggers since the execution
+				 * of the portal may be interleaved with other queries.
+				 */
+
 				/*
 				 * Call ExecStart to prepare the plan for execution
 				 */
@@ -1144,8 +1160,8 @@ DoPortalRunFetch(Portal portal,
 				return PortalRunSelect(portal, false, 1L, dest);
 			}
 			else
-/* count == 0 */
 			{
+				/* count == 0 */
 				/* Rewind to start, return zero rows */
 				DoPortalRewind(portal);
 				return PortalRunSelect(portal, true, 0L, dest);
@@ -1173,8 +1189,8 @@ DoPortalRunFetch(Portal portal,
 				return PortalRunSelect(portal, false, 1L, dest);
 			}
 			else
-/* count == 0 */
 			{
+				/* count == 0 */
 				/* Same as FETCH FORWARD 0, so fall out of switch */
 				fdirection = FETCH_FORWARD;
 			}

src/backend/tcop/utility.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.228 2004/08/29 05:06:49 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.229 2004/09/10 18:40:00 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -912,7 +912,7 @@ ProcessUtility(Node *parsetree,
 			break;
 
 		case T_ConstraintsSetStmt:
-			DeferredTriggerSetState((ConstraintsSetStmt *) parsetree);
+			AfterTriggerSetState((ConstraintsSetStmt *) parsetree);
 			break;
 
 		case T_CreateGroupStmt:

src/backend/utils/adt/ri_triggers.c

@@ -17,7 +17,7 @@
  *
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.71 2004/08/29 05:06:49 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.72 2004/09/10 18:40:04 tgl Exp $
  *
  * ----------
  */
@@ -2454,7 +2454,7 @@ RI_FKey_setdefault_upd(PG_FUNCTION_ARGS)
  *
  *	Check if we have a key change on update.
  *
- *	This is not a real trigger procedure. It is used by the deferred
+ *	This is not a real trigger procedure. It is used by the AFTER
  *	trigger queue manager to detect "triggered data change violation".
  * ----------
  */