Redesign query-snapshot timing so that volatile functions in READ COMMITTED

mode see a fresh snapshot for each command in the function, rather than
using the latest interactive command's snapshot.  Also, suppress fresh
snapshots as well as CommandCounterIncrement inside STABLE and IMMUTABLE
functions, instead using the snapshot taken for the most closely nested
regular query.  (This behavior is only sane for read-only functions, so
the patch also enforces that such functions contain only SELECT commands.)
As per my proposal of 6-Sep-2004; I note that I floated essentially the
same proposal on 19-Jun-2002, but that discussion tailed off without any
action.  Since 8.0 seems like the right place to be taking possibly
nontrivial backwards compatibility hits, let's get it done now.

3 files changed, 107 insertions, 95 deletions

diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 9c32d57c111..20ad56c31f1 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/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.72 2004/09/10 18:40:04 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ri_triggers.c,v 1.73 2004/09/13 20:07:13 tgl Exp $
  *
  * ----------
  */
@@ -2698,16 +2698,20 @@ RI_Initial_Check(FkConstraint *fkconstraint, Relation rel, Relation pkrel)
 		elog(ERROR, "SPI_prepare returned %d for %s", SPI_result, querystr);
 
 	/*
-	 * Run the plan.  For safety we force a current query snapshot to be
-	 * used.  (In serializable mode, this arguably violates
-	 * serializability, but we really haven't got much choice.)  We need
-	 * at most one tuple returned, so pass limit = 1.
+	 * Run the plan.  For safety we force a current snapshot to be used.
+	 * (In serializable mode, this arguably violates serializability, but we
+	 * really haven't got much choice.)  We need at most one tuple returned,
+	 * so pass limit = 1.
 	 */
-	spi_result = SPI_execp_current(qplan, NULL, NULL, true, 1);
+	spi_result = SPI_execute_snapshot(qplan,
+									  NULL, NULL,
+									  CopySnapshot(GetLatestSnapshot()),
+									  InvalidSnapshot,
+									  true, 1);
 
 	/* Check result */
 	if (spi_result != SPI_OK_SELECT)
-		elog(ERROR, "SPI_execp_current returned %d", spi_result);
+		elog(ERROR, "SPI_execute_snapshot returned %d", spi_result);
 
 	/* Did we find a tuple violating the constraint? */
 	if (SPI_processed > 0)
@@ -3043,7 +3047,8 @@ ri_PerformCheck(RI_QueryKey *qkey, void *qplan,
 	Relation	query_rel,
 				source_rel;
 	int			key_idx;
-	bool		useCurrentSnapshot;
+	Snapshot	test_snapshot;
+	Snapshot	crosscheck_snapshot;
 	int			limit;
 	int			spi_result;
 	AclId		save_uid;
@@ -3094,21 +3099,26 @@ ri_PerformCheck(RI_QueryKey *qkey, void *qplan,
 	}
 
 	/*
-	 * In READ COMMITTED mode, we just need to make sure the regular query
-	 * snapshot is up-to-date, and we will see all rows that could be
-	 * interesting.  In SERIALIZABLE mode, we can't update the regular
-	 * query snapshot.	If the caller passes detectNewRows == false then
-	 * it's okay to do the query with the transaction snapshot; otherwise
-	 * we tell the executor to force a current snapshot (and error out if
-	 * it finds any rows under current snapshot that wouldn't be visible
-	 * per the transaction snapshot).
+	 * In READ COMMITTED mode, we just need to use an up-to-date regular
+	 * snapshot, and we will see all rows that could be interesting.
+	 * But in SERIALIZABLE mode, we can't change the transaction snapshot.
+	 * If the caller passes detectNewRows == false then it's okay to do the
+	 * query with the transaction snapshot; otherwise we use a current
+	 * snapshot, and tell the executor to error out if it finds any rows under
+	 * the current snapshot that wouldn't be visible per the transaction
+	 * snapshot.
 	 */
-	if (IsXactIsoLevelSerializable)
-		useCurrentSnapshot = detectNewRows;
+	if (IsXactIsoLevelSerializable && detectNewRows)
+	{
+		CommandCounterIncrement();	/* be sure all my own work is visible */
+		test_snapshot = CopySnapshot(GetLatestSnapshot());
+		crosscheck_snapshot = CopySnapshot(GetTransactionSnapshot());
+	}
 	else
 	{
-		SetQuerySnapshot();
-		useCurrentSnapshot = false;
+		/* the default SPI behavior is okay */
+		test_snapshot = InvalidSnapshot;
+		crosscheck_snapshot = InvalidSnapshot;
 	}
 
 	/*
@@ -3124,15 +3134,17 @@ ri_PerformCheck(RI_QueryKey *qkey, void *qplan,
 	SetUserId(RelationGetForm(query_rel)->relowner);
 
 	/* Finally we can run the query. */
-	spi_result = SPI_execp_current(qplan, vals, nulls,
-								   useCurrentSnapshot, limit);
+	spi_result = SPI_execute_snapshot(qplan,
+									  vals, nulls,
+									  test_snapshot, crosscheck_snapshot,
+									  false, limit);
 
 	/* Restore UID */
 	SetUserId(save_uid);
 
 	/* Check result */
 	if (spi_result < 0)
-		elog(ERROR, "SPI_execp_current returned %d", spi_result);
+		elog(ERROR, "SPI_execute_snapshot returned %d", spi_result);
 
 	if (expect_OK >= 0 && spi_result != expect_OK)
 		ri_ReportViolation(qkey, constrname ? constrname : "",
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index af859222c36..89a9e2451dd 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -3,7 +3,7 @@
  *				back to source text
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.180 2004/09/01 23:58:38 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.181 2004/09/13 20:07:13 tgl Exp $
  *
  *	  This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -290,7 +290,7 @@ pg_get_ruledef_worker(Oid ruleoid, int prettyFlags)
 	 */
 	args[0] = ObjectIdGetDatum(ruleoid);
 	nulls[0] = ' ';
-	spirc = SPI_execp(plan_getrulebyoid, args, nulls, 1);
+	spirc = SPI_execute_plan(plan_getrulebyoid, args, nulls, true, 1);
 	if (spirc != SPI_OK_SELECT)
 		elog(ERROR, "failed to get pg_rewrite tuple for rule %u", ruleoid);
 	if (SPI_processed != 1)
@@ -425,7 +425,7 @@ pg_get_viewdef_worker(Oid viewoid, int prettyFlags)
 	args[1] = PointerGetDatum(ViewSelectRuleName);
 	nulls[0] = ' ';
 	nulls[1] = ' ';
-	spirc = SPI_execp(plan_getviewrule, args, nulls, 2);
+	spirc = SPI_execute_plan(plan_getviewrule, args, nulls, true, 2);
 	if (spirc != SPI_OK_SELECT)
 		elog(ERROR, "failed to get pg_rewrite tuple for view %u", viewoid);
 	if (SPI_processed != 1)
diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index ffdc4b9e6d9..5df7beaabdd 100644
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -16,7 +16,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/time/tqual.c,v 1.77 2004/08/29 05:06:52 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/time/tqual.c,v 1.78 2004/09/13 20:07:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -28,18 +28,24 @@
 #include "utils/tqual.h"
 
 /*
- * The SnapshotData structs are static to simplify memory allocation
+ * These SnapshotData structs are static to simplify memory allocation
  * (see the hack in GetSnapshotData to avoid repeated malloc/free).
  */
-static SnapshotData QuerySnapshotData;
-static SnapshotData SerializableSnapshotData;
-static SnapshotData CurrentSnapshotData;
 static SnapshotData SnapshotDirtyData;
+static SnapshotData SerializableSnapshotData;
+static SnapshotData LatestSnapshotData;
 
 /* Externally visible pointers to valid snapshots: */
-Snapshot	QuerySnapshot = NULL;
-Snapshot	SerializableSnapshot = NULL;
 Snapshot	SnapshotDirty = &SnapshotDirtyData;
+Snapshot	SerializableSnapshot = NULL;
+Snapshot	LatestSnapshot = NULL;
+
+/*
+ * This pointer is not maintained by this module, but it's convenient
+ * to declare it here anyway.  Callers typically assign a copy of
+ * GetTransactionSnapshot's result to ActiveSnapshot.
+ */
+Snapshot	ActiveSnapshot = NULL;
 
 /* These are updated by GetSnapshotData: */
 TransactionId RecentXmin = InvalidTransactionId;
@@ -1028,101 +1034,94 @@ HeapTupleSatisfiesVacuum(HeapTupleHeader tuple, TransactionId OldestXmin)
 
 
 /*
- * SetQuerySnapshot
- *		Initialize query snapshot for a new query
+ * GetTransactionSnapshot
+ *		Get the appropriate snapshot for a new query in a transaction.
  *
  * The SerializableSnapshot is the first one taken in a transaction.
  * In serializable mode we just use that one throughout the transaction.
- * In read-committed mode, we take a new snapshot at the start of each query.
+ * In read-committed mode, we take a new snapshot each time we are called.
+ *
+ * Note that the return value points at static storage that will be modified
+ * by future calls and by CommandCounterIncrement().  Callers should copy
+ * the result with CopySnapshot() if it is to be used very long.
  */
-void
-SetQuerySnapshot(void)
+Snapshot
+GetTransactionSnapshot(void)
 {
-	/* 1st call in xaction? */
+	/* First call in transaction? */
 	if (SerializableSnapshot == NULL)
 	{
 		SerializableSnapshot = GetSnapshotData(&SerializableSnapshotData, true);
-		QuerySnapshot = SerializableSnapshot;
-		Assert(QuerySnapshot != NULL);
-		return;
+		return SerializableSnapshot;
 	}
 
 	if (IsXactIsoLevelSerializable)
-		QuerySnapshot = SerializableSnapshot;
-	else
-		QuerySnapshot = GetSnapshotData(&QuerySnapshotData, false);
+		return SerializableSnapshot;
 
-	Assert(QuerySnapshot != NULL);
+	LatestSnapshot = GetSnapshotData(&LatestSnapshotData, false);
+
+	return LatestSnapshot;
 }
 
 /*
- * CopyQuerySnapshot
- *		Copy the current query snapshot.
- *
- * Copying the snapshot is done so that a query is guaranteed to use a
- * consistent snapshot for its entire execution life, even if the command
- * counter is incremented or SetQuerySnapshot() is called while it runs
- * (as could easily happen, due to triggers etc. executing queries).
- *
- * The copy is palloc'd in the current memory context.
+ * GetLatestSnapshot
+ *		Get a snapshot that is up-to-date as of the current instant,
+ *		even if we are executing in SERIALIZABLE mode.
  */
 Snapshot
-CopyQuerySnapshot(void)
+GetLatestSnapshot(void)
 {
-	Snapshot	snapshot;
-
-	if (QuerySnapshot == NULL)	/* should be set beforehand */
+	/* Should not be first call in transaction */
+	if (SerializableSnapshot == NULL)
 		elog(ERROR, "no snapshot has been set");
 
-	snapshot = (Snapshot) palloc(sizeof(SnapshotData));
-	memcpy(snapshot, QuerySnapshot, sizeof(SnapshotData));
-	if (snapshot->xcnt > 0)
-	{
-		snapshot->xip = (TransactionId *)
-			palloc(snapshot->xcnt * sizeof(TransactionId));
-		memcpy(snapshot->xip, QuerySnapshot->xip,
-			   snapshot->xcnt * sizeof(TransactionId));
-	}
-	else
-		snapshot->xip = NULL;
+	LatestSnapshot = GetSnapshotData(&LatestSnapshotData, false);
 
-	return snapshot;
+	return LatestSnapshot;
 }
 
 /*
- * CopyCurrentSnapshot
- *		Make a snapshot that is up-to-date as of the current instant,
- *		and return a copy.
+ * CopySnapshot
+ *		Copy the given snapshot.
  *
  * The copy is palloc'd in the current memory context.
+ *
+ * Note that this will not work on "special" snapshots.
  */
 Snapshot
-CopyCurrentSnapshot(void)
+CopySnapshot(Snapshot snapshot)
 {
-	Snapshot	currentSnapshot;
-	Snapshot	snapshot;
-
-	if (QuerySnapshot == NULL)	/* should not be first call in xact */
-		elog(ERROR, "no snapshot has been set");
-
-	/* Update the static struct */
-	currentSnapshot = GetSnapshotData(&CurrentSnapshotData, false);
-	currentSnapshot->curcid = GetCurrentCommandId();
+	Snapshot	newsnap;
 
-	/* Make a copy */
-	snapshot = (Snapshot) palloc(sizeof(SnapshotData));
-	memcpy(snapshot, currentSnapshot, sizeof(SnapshotData));
+	/* We allocate any XID array needed in the same palloc block. */
+	newsnap = (Snapshot) palloc(sizeof(SnapshotData) +
+								snapshot->xcnt * sizeof(TransactionId));
+	memcpy(newsnap, snapshot, sizeof(SnapshotData));
 	if (snapshot->xcnt > 0)
 	{
-		snapshot->xip = (TransactionId *)
-			palloc(snapshot->xcnt * sizeof(TransactionId));
-		memcpy(snapshot->xip, currentSnapshot->xip,
+		newsnap->xip = (TransactionId *) (newsnap + 1);
+		memcpy(newsnap->xip, snapshot->xip,
 			   snapshot->xcnt * sizeof(TransactionId));
 	}
 	else
-		snapshot->xip = NULL;
+		newsnap->xip = NULL;
 
-	return snapshot;
+	return newsnap;
+}
+
+/*
+ * FreeSnapshot
+ *		Free a snapshot previously copied with CopySnapshot.
+ *
+ * This is currently identical to pfree, but is provided for cleanliness.
+ *
+ * Do *not* apply this to the results of GetTransactionSnapshot or
+ * GetLatestSnapshot.
+ */
+void
+FreeSnapshot(Snapshot snapshot)
+{
+	pfree(snapshot);
 }
 
 /*
@@ -1133,10 +1132,11 @@ void
 FreeXactSnapshot(void)
 {
 	/*
-	 * We do not free the xip arrays for the snapshot structs; they will
-	 * be reused soon.	So this is now just a state change to prevent
+	 * We do not free the xip arrays for the static snapshot structs; they
+	 * will be reused soon. So this is now just a state change to prevent
 	 * outside callers from accessing the snapshots.
 	 */
-	QuerySnapshot = NULL;
 	SerializableSnapshot = NULL;
+	LatestSnapshot = NULL;
+	ActiveSnapshot = NULL;		/* just for cleanliness */
 }