RecentXmin is too recent to use as the cutoff point for accessing

pg_subtrans --- what we need is the oldest xmin of any snapshot in use
in the current top transaction.  Introduce a new variable TransactionXmin
to play this role.  Fixes intermittent regression failure reported by
Neil Conway.

src/backend/access/transam/subtrans.c

@@ -22,7 +22,7 @@
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/access/transam/subtrans.c,v 1.5 2004/08/29 05:06:40 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/subtrans.c,v 1.6 2004/09/16 18:35:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -105,7 +105,7 @@ SubTransGetParent(TransactionId xid)
 	TransactionId parent;
 
 	/* Can't ask about stuff that might not be around anymore */
-	Assert(TransactionIdFollowsOrEquals(xid, RecentXmin));
+	Assert(TransactionIdFollowsOrEquals(xid, TransactionXmin));
 
 	/* Bootstrap and frozen XIDs have no parent */
 	if (!TransactionIdIsNormal(xid))
@@ -129,12 +129,12 @@ SubTransGetParent(TransactionId xid)
  *
  * Returns the topmost transaction of the given transaction id.
  *
- * Because we cannot look back further than RecentXmin, it is possible
+ * Because we cannot look back further than TransactionXmin, it is possible
  * that this function will lie and return an intermediate subtransaction ID
  * instead of the true topmost parent ID.  This is OK, because in practice
  * we only care about detecting whether the topmost parent is still running
  * or is part of a current snapshot's list of still-running transactions.
- * Therefore, any XID before RecentXmin is as good as any other.
+ * Therefore, any XID before TransactionXmin is as good as any other.
  */
 TransactionId
 SubTransGetTopmostTransaction(TransactionId xid)
@@ -143,12 +143,12 @@ SubTransGetTopmostTransaction(TransactionId xid)
 				previousXid = xid;
 
 	/* Can't ask about stuff that might not be around anymore */
-	Assert(TransactionIdFollowsOrEquals(xid, RecentXmin));
+	Assert(TransactionIdFollowsOrEquals(xid, TransactionXmin));
 
 	while (TransactionIdIsValid(parentXid))
 	{
 		previousXid = parentXid;
-		if (TransactionIdPrecedes(parentXid, RecentXmin))
+		if (TransactionIdPrecedes(parentXid, TransactionXmin))
 			break;
 		parentXid = SubTransGetParent(parentXid);
 	}
@@ -312,7 +312,7 @@ ExtendSUBTRANS(TransactionId newestXact)
  * Remove all SUBTRANS segments before the one holding the passed transaction ID
  *
  * This is normally called during checkpoint, with oldestXact being the
- * oldest XMIN of any running transaction.
+ * oldest TransactionXmin of any running transaction.
  */
 void
 TruncateSUBTRANS(TransactionId oldestXact)

src/backend/access/transam/transam.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/transam/transam.c,v 1.61 2004/08/29 05:06:40 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/transam/transam.c,v 1.62 2004/09/16 18:35:20 tgl Exp $
  *
  * NOTES
  *	  This file contains the high level access-method interface to the
@@ -200,15 +200,15 @@ TransactionIdDidCommit(TransactionId transactionId)
 
 	/*
 	 * If it's marked subcommitted, we have to check the parent
-	 * recursively. However, if it's older than RecentXmin, we can't look
-	 * at pg_subtrans; instead assume that the parent crashed without
+	 * recursively. However, if it's older than TransactionXmin, we can't
+	 * look at pg_subtrans; instead assume that the parent crashed without
 	 * cleaning up its children.
 	 */
 	if (xidstatus == TRANSACTION_STATUS_SUB_COMMITTED)
 	{
 		TransactionId parentXid;
 
-		if (TransactionIdPrecedes(transactionId, RecentXmin))
+		if (TransactionIdPrecedes(transactionId, TransactionXmin))
 			return false;
 		parentXid = SubTransGetParent(transactionId);
 		Assert(TransactionIdIsValid(parentXid));
@@ -249,15 +249,15 @@ TransactionIdDidAbort(TransactionId transactionId)
 
 	/*
 	 * If it's marked subcommitted, we have to check the parent
-	 * recursively. However, if it's older than RecentXmin, we can't look
-	 * at pg_subtrans; instead assume that the parent crashed without
+	 * recursively. However, if it's older than TransactionXmin, we can't
+	 * look at pg_subtrans; instead assume that the parent crashed without
 	 * cleaning up its children.
 	 */
 	if (xidstatus == TRANSACTION_STATUS_SUB_COMMITTED)
 	{
 		TransactionId parentXid;
 
-		if (TransactionIdPrecedes(transactionId, RecentXmin))
+		if (TransactionIdPrecedes(transactionId, TransactionXmin))
 			return true;
 		parentXid = SubTransGetParent(transactionId);
 		Assert(TransactionIdIsValid(parentXid));

src/backend/storage/ipc/sinval.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/ipc/sinval.c,v 1.73 2004/09/06 23:33:35 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/ipc/sinval.c,v 1.74 2004/09/16 18:35:21 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -521,7 +521,8 @@ TransactionIdIsInProgress(TransactionId xid)
 	bool		locked;
 
 	/*
-	 * Don't bother checking a very old transaction.
+	 * Don't bother checking a transaction older than RecentXmin; it
+	 * could not possibly still be running.
 	 */
 	if (TransactionIdPrecedes(xid, RecentXmin))
 	{
@@ -732,10 +733,19 @@ GetOldestXmin(bool allDbs)
  * This ensures that the set of transactions seen as "running" by the
  * current xact will not change after it takes the snapshot.
  *
- * We also compute the current global xmin (oldest xmin across all running
- * transactions) and save it in RecentGlobalXmin.  This is the same
- * computation done by GetOldestXmin(TRUE).  The xmin value is also stored
- * into RecentXmin.
+ * Note that only top-level XIDs are included in the snapshot.  We can
+ * still apply the xmin and xmax limits to subtransaction XIDs, but we
+ * need to work a bit harder to see if XIDs in [xmin..xmax) are running.
+ *
+ * We also update the following backend-global variables:
+ *		TransactionXmin: the oldest xmin of any snapshot in use in the
+ *			current transaction (this is the same as MyProc->xmin).  This
+ *			is just the xmin computed for the first, serializable snapshot.
+ *		RecentXmin: the xmin computed for the most recent snapshot.  XIDs
+ *			older than this are known not running any more.
+ *		RecentGlobalXmin: the global xmin (oldest TransactionXmin across all
+ *			running transactions).  This is the same computation done by
+ *			GetOldestXmin(TRUE).
  *----------
  */
 Snapshot
@@ -751,6 +761,11 @@ GetSnapshotData(Snapshot snapshot, bool serializable)
 
 	Assert(snapshot != NULL);
 
+	/* Serializable snapshot must be computed before any other... */
+	Assert(serializable ?
+		   !TransactionIdIsValid(MyProc->xmin) :
+		   TransactionIdIsValid(MyProc->xmin));
+
 	/*
 	 * Allocating space for MaxBackends xids is usually overkill;
 	 * lastBackend would be sufficient.  But it seems better to do the
@@ -850,13 +865,10 @@ GetSnapshotData(Snapshot snapshot, bool serializable)
 	}
 
 	if (serializable)
-		MyProc->xmin = xmin;
+		MyProc->xmin = TransactionXmin = xmin;
 
 	LWLockRelease(SInvalLock);
 
-	/* Serializable snapshot must be computed before any other... */
-	Assert(TransactionIdIsValid(MyProc->xmin));
-
 	/*
 	 * Update globalxmin to include actual process xids.  This is a
 	 * slightly different way of computing it than GetOldestXmin uses, but
@@ -865,7 +877,7 @@ GetSnapshotData(Snapshot snapshot, bool serializable)
 	if (TransactionIdPrecedes(xmin, globalxmin))
 		globalxmin = xmin;
 
-	/* Update globals for use by VACUUM */
+	/* Update global variables too */
 	RecentGlobalXmin = globalxmin;
 	RecentXmin = xmin;
 

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.78 2004/09/13 20:07:36 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/time/tqual.c,v 1.79 2004/09/16 18:35:22 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -48,6 +48,7 @@ Snapshot	LatestSnapshot = NULL;
 Snapshot	ActiveSnapshot = NULL;
 
 /* These are updated by GetSnapshotData: */
+TransactionId TransactionXmin = InvalidTransactionId;
 TransactionId RecentXmin = InvalidTransactionId;
 TransactionId RecentGlobalXmin = InvalidTransactionId;
 

src/include/utils/tqual.h

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/utils/tqual.h,v 1.52 2004/09/13 20:08:35 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/utils/tqual.h,v 1.53 2004/09/16 18:35:23 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -59,6 +59,7 @@ extern DLLIMPORT Snapshot SerializableSnapshot;
 extern DLLIMPORT Snapshot LatestSnapshot;
 extern DLLIMPORT Snapshot ActiveSnapshot;
 
+extern TransactionId TransactionXmin;
 extern TransactionId RecentXmin;
 extern TransactionId RecentGlobalXmin;