Remove global variable scanCommandId in favor of storing a command ID

in snapshots, per my proposal of a few days ago.  Also, tweak heapam.c
routines (heap_insert, heap_update, heap_delete, heap_mark4update) to
be passed the command ID to use, instead of doing GetCurrentCommandID.
For catalog updates they'll still get passed current command ID, but
for updates generated from the main executor they'll get passed the
command ID saved in the snapshot the query is using.  This should fix
some corner cases associated with functions and triggers that advance
current command ID while an outer query is still in progress.

3 files changed, 50 insertions, 59 deletions

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index dac14ac3397..a8d7ca0c29f 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.134 2002/05/20 23:51:41 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.135 2002/05/21 22:05:53 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1059,15 +1059,14 @@ heap_get_latest_tid(Relation relation,
 	return tid;
 }
 
-/* ----------------
- *		heap_insert		- insert tuple into a heap
+/*
+ *	heap_insert		- insert tuple into a heap
  *
- *		The assignment of t_min (and thus the others) should be
- *		removed eventually.
- * ----------------
+ * The new tuple is stamped with current transaction ID and the specified
+ * command ID.
  */
 Oid
-heap_insert(Relation relation, HeapTuple tup)
+heap_insert(Relation relation, HeapTuple tup, CommandId cid)
 {
 	Buffer		buffer;
 
@@ -1093,8 +1092,9 @@ heap_insert(Relation relation, HeapTuple tup)
 	}
 
 	TransactionIdStore(GetCurrentTransactionId(), &(tup->t_data->t_xmin));
-	tup->t_data->t_cmin = GetCurrentCommandId();
+	tup->t_data->t_cmin = cid;
 	StoreInvalidTransactionId(&(tup->t_data->t_xmax));
+	tup->t_data->t_cmax = FirstCommandId;
 	tup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
 	tup->t_data->t_infomask |= HEAP_XMAX_INVALID;
 	tup->t_tableOid = relation->rd_id;
@@ -1179,13 +1179,27 @@ heap_insert(Relation relation, HeapTuple tup)
 }
 
 /*
+ *	simple_heap_insert - insert a tuple
+ *
+ * Currently, this routine differs from heap_insert only in supplying
+ * a default command ID.  But it should be used rather than using
+ * heap_insert directly in most places where we are modifying system catalogs.
+ */
+Oid
+simple_heap_insert(Relation relation, HeapTuple tup)
+{
+	return heap_insert(relation, tup, GetCurrentCommandId());
+}
+
+/*
  *	heap_delete		- delete a tuple
  *
  * NB: do not call this directly unless you are prepared to deal with
  * concurrent-update conditions.  Use simple_heap_delete instead.
  */
 int
-heap_delete(Relation relation, ItemPointer tid, ItemPointer ctid)
+heap_delete(Relation relation, ItemPointer tid,
+			ItemPointer ctid, CommandId cid)
 {
 	ItemId		lp;
 	HeapTupleData tp;
@@ -1215,7 +1229,7 @@ heap_delete(Relation relation, ItemPointer tid, ItemPointer ctid)
 	tp.t_tableOid = relation->rd_id;
 
 l1:
-	result = HeapTupleSatisfiesUpdate(&tp);
+	result = HeapTupleSatisfiesUpdate(&tp, cid);
 
 	if (result == HeapTupleInvisible)
 	{
@@ -1265,7 +1279,7 @@ l1:
 	START_CRIT_SECTION();
 	/* store transaction information of xact deleting the tuple */
 	TransactionIdStore(GetCurrentTransactionId(), &(tp.t_data->t_xmax));
-	tp.t_data->t_cmax = GetCurrentCommandId();
+	tp.t_data->t_cmax = cid;
 	tp.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
 							 HEAP_XMAX_INVALID | HEAP_MARKED_FOR_UPDATE);
 	/* XLOG stuff */
@@ -1336,7 +1350,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 	ItemPointerData ctid;
 	int			result;
 
-	result = heap_delete(relation, tid, &ctid);
+	result = heap_delete(relation, tid, &ctid, GetCurrentCommandId());
 	switch (result)
 	{
 		case HeapTupleSelfUpdated:
@@ -1356,7 +1370,6 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 			elog(ERROR, "Unknown status %u from heap_delete", result);
 			break;
 	}
-
 }
 
 /*
@@ -1367,7 +1380,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
  */
 int
 heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
-			ItemPointer ctid)
+			ItemPointer ctid, CommandId cid)
 {
 	ItemId		lp;
 	HeapTupleData oldtup;
@@ -1407,7 +1420,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 	 */
 
 l2:
-	result = HeapTupleSatisfiesUpdate(&oldtup);
+	result = HeapTupleSatisfiesUpdate(&oldtup, cid);
 
 	if (result == HeapTupleInvisible)
 	{
@@ -1457,7 +1470,7 @@ l2:
 	/* Fill in OID and transaction status data for newtup */
 	newtup->t_data->t_oid = oldtup.t_data->t_oid;
 	TransactionIdStore(GetCurrentTransactionId(), &(newtup->t_data->t_xmin));
-	newtup->t_data->t_cmin = GetCurrentCommandId();
+	newtup->t_data->t_cmin = cid;
 	StoreInvalidTransactionId(&(newtup->t_data->t_xmax));
 	newtup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
 	newtup->t_data->t_infomask |= (HEAP_XMAX_INVALID | HEAP_UPDATED);
@@ -1496,7 +1509,7 @@ l2:
 
 		TransactionIdStore(GetCurrentTransactionId(),
 						   &(oldtup.t_data->t_xmax));
-		oldtup.t_data->t_cmax = GetCurrentCommandId();
+		oldtup.t_data->t_cmax = cid;
 		oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
 									   HEAP_XMAX_INVALID |
 									   HEAP_MARKED_FOR_UPDATE);
@@ -1588,7 +1601,7 @@ l2:
 	{
 		TransactionIdStore(GetCurrentTransactionId(),
 						   &(oldtup.t_data->t_xmax));
-		oldtup.t_data->t_cmax = GetCurrentCommandId();
+		oldtup.t_data->t_cmax = cid;
 		oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED |
 									   HEAP_XMAX_INVALID |
 									   HEAP_MARKED_FOR_UPDATE);
@@ -1653,7 +1666,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup)
 	ItemPointerData ctid;
 	int			result;
 
-	result = heap_update(relation, otid, tup, &ctid);
+	result = heap_update(relation, otid, tup, &ctid, GetCurrentCommandId());
 	switch (result)
 	{
 		case HeapTupleSelfUpdated:
@@ -1679,7 +1692,8 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup)
  *	heap_mark4update		- mark a tuple for update
  */
 int
-heap_mark4update(Relation relation, HeapTuple tuple, Buffer *buffer)
+heap_mark4update(Relation relation, HeapTuple tuple, Buffer *buffer,
+				 CommandId cid)
 {
 	ItemPointer tid = &(tuple->t_self);
 	ItemId		lp;
@@ -1704,7 +1718,7 @@ heap_mark4update(Relation relation, HeapTuple tuple, Buffer *buffer)
 	tuple->t_len = ItemIdGetLength(lp);
 
 l3:
-	result = HeapTupleSatisfiesUpdate(tuple);
+	result = HeapTupleSatisfiesUpdate(tuple, cid);
 
 	if (result == HeapTupleInvisible)
 	{
@@ -1758,7 +1772,7 @@ l3:
 
 	/* store transaction information of xact marking the tuple */
 	TransactionIdStore(GetCurrentTransactionId(), &(tuple->t_data->t_xmax));
-	tuple->t_data->t_cmax = GetCurrentCommandId();
+	tuple->t_data->t_cmax = cid;
 	tuple->t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED | HEAP_XMAX_INVALID);
 	tuple->t_data->t_infomask |= HEAP_MARKED_FOR_UPDATE;
 
@@ -2400,9 +2414,8 @@ _heap_unlock_tuple(void *data)
 
 	htup = (HeapTupleHeader) PageGetItem(page, lp);
 
-	if (!TransactionIdEquals(htup->t_xmax, GetCurrentTransactionId()) ||
-		htup->t_cmax != GetCurrentCommandId())
-		elog(PANIC, "_heap_unlock_tuple: invalid xmax/cmax in rollback");
+	if (!TransactionIdEquals(htup->t_xmax, GetCurrentTransactionId()))
+		elog(PANIC, "_heap_unlock_tuple: invalid xmax in rollback");
 	htup->t_infomask &= ~HEAP_XMAX_UNLOGGED;
 	htup->t_infomask |= HEAP_XMAX_INVALID;
 	UnlockAndWriteBuffer(buffer);
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 18bc99982a5..9ac1c69c8ea 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/heap/tuptoaster.c,v 1.29 2002/05/20 23:51:41 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/heap/tuptoaster.c,v 1.30 2002/05/21 22:05:53 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -911,7 +911,7 @@ toast_save_datum(Relation rel, Datum value)
 		if (!HeapTupleIsValid(toasttup))
 			elog(ERROR, "Failed to build TOAST tuple");
 
-		heap_insert(toastrel, toasttup);
+		simple_heap_insert(toastrel, toasttup);
 
 		/*
 		 * Create the index entry.	We cheat a little here by not using
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 39e4afeb749..10de2f8a6d5 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.122 2002/05/17 20:53:33 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/transam/xact.c,v 1.123 2002/05/21 22:05:53 tgl Exp $
  *
  * NOTES
  *		Transaction aborts can now occur two ways:
@@ -350,14 +350,6 @@ GetCurrentCommandId(void)
 	return s->commandId;
 }
 
-CommandId
-GetScanCommandId(void)
-{
-	TransactionState s = CurrentTransactionState;
-
-	return s->scanCommandId;
-}
-
 
 /* --------------------------------
  *		GetCurrentTransactionStartTime
@@ -418,17 +410,6 @@ CommandIdIsCurrentCommandId(CommandId cid)
 	return (cid == s->commandId) ? true : false;
 }
 
-bool
-CommandIdGEScanCommandId(CommandId cid)
-{
-	TransactionState s = CurrentTransactionState;
-
-	if (AMI_OVERRIDE)
-		return false;
-
-	return (cid >= s->scanCommandId) ? true : false;
-}
-
 
 /* --------------------------------
  *		CommandCounterIncrement
@@ -437,11 +418,17 @@ CommandIdGEScanCommandId(CommandId cid)
 void
 CommandCounterIncrement(void)
 {
-	CurrentTransactionStateData.commandId += 1;
-	if (CurrentTransactionStateData.commandId == FirstCommandId)
+	TransactionState s = CurrentTransactionState;
+
+	s->commandId += 1;
+	if (s->commandId == FirstCommandId)	/* check for overflow */
 		elog(ERROR, "You may only have 2^32-1 commands per transaction");
 
-	CurrentTransactionStateData.scanCommandId = CurrentTransactionStateData.commandId;
+	/* Propagate new command ID into query snapshots, if set */
+	if (QuerySnapshot)
+		QuerySnapshot->curcid = s->commandId;
+	if (SerializableSnapshot)
+		SerializableSnapshot->curcid = s->commandId;
 
 	/*
 	 * make cache changes visible to me.  AtCommit_LocalCache() instead of
@@ -451,11 +438,6 @@ CommandCounterIncrement(void)
 	AtStart_Cache();
 }
 
-void
-SetScanCommandId(CommandId savedId)
-{
-	CurrentTransactionStateData.scanCommandId = savedId;
-}
 
 /* ----------------------------------------------------------------
  *						StartTransaction stuff
@@ -889,10 +871,6 @@ StartTransaction(void)
 	 * initialize current transaction state fields
 	 */
 	s->commandId = FirstCommandId;
-	s->scanCommandId = FirstCommandId;
-#if NOT_USED
-	s->startTime = GetCurrentAbsoluteTime();
-#endif
 	s->startTime = GetCurrentAbsoluteTimeUsec(&(s->startTimeUsec));
 
 	/*