Avoid incrementing the CommandCounter when CommandCounterIncrement is called

but no database changes have been made since the last CommandCounterIncrement.
This should result in a significant improvement in the number of "commands"
that can typically be performed within a transaction before hitting the 2^32
CommandId size limit.  In particular this buys back (and more) the possible
adverse consequences of my previous patch to fix plan caching behavior.

The implementation requires tracking whether the current CommandCounter
value has been "used" to mark any tuples.  CommandCounter values stored into
snapshots are presumed not to be used for this purpose.  This requires some
small executor changes, since the executor used to conflate the curcid of
the snapshot it was using with the command ID to mark output tuples with.
Separating these concepts allows some small simplifications in executor APIs.

Something for the TODO list: look into having CommandCounterIncrement not do
AcceptInvalidationMessages.  It seems fairly bogus to be doing it there,
but exactly where to do it instead isn't clear, and I'm disinclined to mess
with asynchronous behavior during late beta.

Author: tglsfdc

contrib/pgrowlocks/pgrowlocks.c

@@ -1,5 +1,5 @@
 /*
- * $PostgreSQL: pgsql/contrib/pgrowlocks/pgrowlocks.c,v 1.7 2007/08/28 22:59:30 tgl Exp $
+ * $PostgreSQL: pgsql/contrib/pgrowlocks/pgrowlocks.c,v 1.8 2007/11/30 21:22:53 tgl Exp $
  *
  * Copyright (c) 2005-2006	Tatsuo Ishii
  *
@@ -117,8 +117,9 @@ pgrowlocks(PG_FUNCTION_ARGS)
 		/* must hold a buffer lock to call HeapTupleSatisfiesUpdate */
 		LockBuffer(scan->rs_cbuf, BUFFER_LOCK_SHARE);
 
-		if (HeapTupleSatisfiesUpdate(tuple->t_data, GetCurrentCommandId(), scan->rs_cbuf)
-			== HeapTupleBeingUpdated)
+		if (HeapTupleSatisfiesUpdate(tuple->t_data,
+									 GetCurrentCommandId(false),
+									 scan->rs_cbuf) == HeapTupleBeingUpdated)
 		{
 
 			char	  **values;

src/backend/access/heap/heapam.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.245 2007/11/15 21:14:32 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.246 2007/11/30 21:22:53 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1891,7 +1891,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
 Oid
 simple_heap_insert(Relation relation, HeapTuple tup)
 {
-	return heap_insert(relation, tup, GetCurrentCommandId(), true, true);
+	return heap_insert(relation, tup, GetCurrentCommandId(true), true, true);
 }
 
 /*
@@ -2174,7 +2174,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 
 	result = heap_delete(relation, tid,
 						 &update_ctid, &update_xmax,
-						 GetCurrentCommandId(), InvalidSnapshot,
+						 GetCurrentCommandId(true), InvalidSnapshot,
 						 true /* wait for commit */ );
 	switch (result)
 	{
@@ -2817,7 +2817,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup)
 
 	result = heap_update(relation, otid, tup,
 						 &update_ctid, &update_xmax,
-						 GetCurrentCommandId(), InvalidSnapshot,
+						 GetCurrentCommandId(true), InvalidSnapshot,
 						 true /* wait for commit */ );
 	switch (result)
 	{

src/backend/access/heap/tuptoaster.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.79 2007/11/15 21:14:32 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.80 2007/11/30 21:22:53 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1086,7 +1086,7 @@ toast_save_datum(Relation rel, Datum value,
 	TupleDesc	toasttupDesc;
 	Datum		t_values[3];
 	bool		t_isnull[3];
-	CommandId	mycid = GetCurrentCommandId();
+	CommandId	mycid = GetCurrentCommandId(true);
 	struct varlena *result;
 	struct varatt_external toast_pointer;
 	struct

src/backend/access/transam/xact.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.253 2007/11/15 21:14:32 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.254 2007/11/30 21:22:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -161,6 +161,7 @@ static TransactionState CurrentTransactionState = &TopTransactionStateData;
  */
 static SubTransactionId currentSubTransactionId;
 static CommandId currentCommandId;
+static bool currentCommandIdUsed;
 
 /*
  * xactStartTimestamp is the value of transaction_timestamp().
@@ -435,11 +436,18 @@ GetCurrentSubTransactionId(void)
 
 /*
  *	GetCurrentCommandId
+ *
+ * "used" must be TRUE if the caller intends to use the command ID to mark
+ * inserted/updated/deleted tuples.  FALSE means the ID is being fetched
+ * for read-only purposes (ie, as a snapshot validity cutoff).  See
+ * CommandCounterIncrement() for discussion.
  */
 CommandId
-GetCurrentCommandId(void)
+GetCurrentCommandId(bool used)
 {
 	/* this is global to a transaction, not subtransaction-local */
+	if (used)
+		currentCommandIdUsed = true;
 	return currentCommandId;
 }
 
@@ -566,25 +574,50 @@ TransactionIdIsCurrentTransactionId(TransactionId xid)
 void
 CommandCounterIncrement(void)
 {
-	currentCommandId += 1;
-	if (currentCommandId == FirstCommandId)		/* check for overflow */
+	/*
+	 * If the current value of the command counter hasn't been "used" to
+	 * mark tuples, we need not increment it, since there's no need to
+	 * distinguish a read-only command from others.  This helps postpone
+	 * command counter overflow, and keeps no-op CommandCounterIncrement
+	 * operations cheap.
+	 */
+	if (currentCommandIdUsed)
 	{
-		currentCommandId -= 1;
-		ereport(ERROR,
-				(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+		currentCommandId += 1;
+		if (currentCommandId == FirstCommandId)	/* check for overflow */
+		{
+			currentCommandId -= 1;
+			ereport(ERROR,
+					(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
 		  errmsg("cannot have more than 2^32-1 commands in a transaction")));
-	}
+		}
+		currentCommandIdUsed = false;
 
-	/* Propagate new command ID into static snapshots, if set */
-	if (SerializableSnapshot)
-		SerializableSnapshot->curcid = currentCommandId;
-	if (LatestSnapshot)
-		LatestSnapshot->curcid = currentCommandId;
+		/* Propagate new command ID into static snapshots, if set */
+		if (SerializableSnapshot)
+			SerializableSnapshot->curcid = currentCommandId;
+		if (LatestSnapshot)
+			LatestSnapshot->curcid = currentCommandId;
+
+		/*
+		 * Make any catalog changes done by the just-completed command
+		 * visible in the local syscache.  We obviously don't need to do
+		 * this after a read-only command.  (But see hacks in inval.c
+		 * to make real sure we don't think a command that queued inval
+		 * messages was read-only.)
+		 */
+		AtCommit_LocalCache();
+	}
 
 	/*
-	 * make cache changes visible to me.
+	 * Make any other backends' catalog changes visible to me.
+	 *
+	 * XXX this is probably in the wrong place: CommandCounterIncrement
+	 * should be purely a local operation, most likely.  However fooling
+	 * with this will affect asynchronous cross-backend interactions,
+	 * which doesn't seem like a wise thing to do in late beta, so save
+	 * improving this for another day - tgl 2007-11-30
 	 */
-	AtCommit_LocalCache();
 	AtStart_Cache();
 }
 
@@ -1416,6 +1449,7 @@ StartTransaction(void)
 	s->subTransactionId = TopSubTransactionId;
 	currentSubTransactionId = TopSubTransactionId;
 	currentCommandId = FirstCommandId;
+	currentCommandIdUsed = false;
 
 	/*
 	 * must initialize resource-management stuff first
@@ -4007,13 +4041,14 @@ ShowTransactionStateRec(TransactionState s)
 
 	/* use ereport to suppress computation if msg will not be printed */
 	ereport(DEBUG3,
-			(errmsg_internal("name: %s; blockState: %13s; state: %7s, xid/subid/cid: %u/%u/%u, nestlvl: %d, children: %s",
+			(errmsg_internal("name: %s; blockState: %13s; state: %7s, xid/subid/cid: %u/%u/%u%s, nestlvl: %d, children: %s",
 							 PointerIsValid(s->name) ? s->name : "unnamed",
 							 BlockStateAsString(s->blockState),
 							 TransStateAsString(s->state),
 							 (unsigned int) s->transactionId,
 							 (unsigned int) s->subTransactionId,
 							 (unsigned int) currentCommandId,
+							 currentCommandIdUsed ? " (used)" : "",
 							 s->nestingLevel,
 							 nodeToString(s->childXids))));
 }

src/backend/commands/async.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.136 2007/04/12 06:53:46 neilc Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/async.c,v 1.137 2007/11/30 21:22:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -562,7 +562,8 @@ AtCommit_Notify(void)
 				 */
 				result = heap_update(lRel, &lTuple->t_self, rTuple,
 									 &update_ctid, &update_xmax,
-									 GetCurrentCommandId(), InvalidSnapshot,
+									 GetCurrentCommandId(true),
+									 InvalidSnapshot,
 									 false /* no wait for commit */ );
 				switch (result)
 				{

src/backend/commands/copy.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.288 2007/11/15 21:14:33 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/copy.c,v 1.289 2007/11/30 21:22:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1033,7 +1033,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString)
 		 * which COPY can be invoked, I think it's OK, because the active
 		 * snapshot shouldn't be shared with anything else anyway.)
 		 */
-		ActiveSnapshot->curcid = GetCurrentCommandId();
+		ActiveSnapshot->curcid = GetCurrentCommandId(false);
 
 		/* Create dest receiver for COPY OUT */
 		dest = CreateDestReceiver(DestCopyOut, NULL);
@@ -1637,7 +1637,7 @@ CopyFrom(CopyState cstate)
 	ExprContext *econtext;		/* used for ExecEvalExpr for default atts */
 	MemoryContext oldcontext = CurrentMemoryContext;
 	ErrorContextCallback errcontext;
-	CommandId	mycid = GetCurrentCommandId();
+	CommandId	mycid = GetCurrentCommandId(true);
 	bool		use_wal = true; /* by default, use WAL logging */
 	bool		use_fsm = true; /* by default, use FSM for free space */
 

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.167 2007/11/15 22:25:15 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/explain.c,v 1.168 2007/11/30 21:22:54 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -233,7 +233,7 @@ ExplainOnePlan(PlannedStmt *plannedstmt, ParamListInfo params,
 	 * EXPLAIN can be invoked, I think it's OK, because the active snapshot
 	 * shouldn't be shared with anything else anyway.)
 	 */
-	ActiveSnapshot->curcid = GetCurrentCommandId();
+	ActiveSnapshot->curcid = GetCurrentCommandId(false);
 
 	/* Create a QueryDesc requesting no output */
 	queryDesc = CreateQueryDesc(plannedstmt,

src/backend/commands/trigger.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.224 2007/11/16 01:51:22 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.225 2007/11/30 21:22:54 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -51,7 +51,6 @@ static void InsertTrigger(TriggerDesc *trigdesc, Trigger *trigger, int indx);
 static HeapTuple GetTupleForTrigger(EState *estate,
 				   ResultRelInfo *relinfo,
 				   ItemPointer tid,
-				   CommandId cid,
 				   TupleTableSlot **newSlot);
 static HeapTuple ExecCallTriggerFunc(TriggerData *trigdata,
 					int tgindx,
@@ -1801,8 +1800,7 @@ ExecASDeleteTriggers(EState *estate, ResultRelInfo *relinfo)
 
 bool
 ExecBRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
-					 ItemPointer tupleid,
-					 CommandId cid)
+					 ItemPointer tupleid)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
 	int			ntrigs = trigdesc->n_before_row[TRIGGER_EVENT_DELETE];
@@ -1814,7 +1812,7 @@ ExecBRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
 	TupleTableSlot *newSlot;
 	int			i;
 
-	trigtuple = GetTupleForTrigger(estate, relinfo, tupleid, cid, &newSlot);
+	trigtuple = GetTupleForTrigger(estate, relinfo, tupleid, &newSlot);
 	if (trigtuple == NULL)
 		return false;
 
@@ -1871,9 +1869,7 @@ ExecARDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
 	if (trigdesc && trigdesc->n_after_row[TRIGGER_EVENT_DELETE] > 0)
 	{
 		HeapTuple	trigtuple = GetTupleForTrigger(estate, relinfo,
-												   tupleid,
-												   (CommandId) 0,
-												   NULL);
+												   tupleid, NULL);
 
 		AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_DELETE,
 							  true, trigtuple, NULL);
@@ -1952,8 +1948,7 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo)
 
 HeapTuple
 ExecBRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
-					 ItemPointer tupleid, HeapTuple newtuple,
-					 CommandId cid)
+					 ItemPointer tupleid, HeapTuple newtuple)
 {
 	TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
 	int			ntrigs = trigdesc->n_before_row[TRIGGER_EVENT_UPDATE];
@@ -1965,7 +1960,7 @@ ExecBRUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
 	TupleTableSlot *newSlot;
 	int			i;
 
-	trigtuple = GetTupleForTrigger(estate, relinfo, tupleid, cid, &newSlot);
+	trigtuple = GetTupleForTrigger(estate, relinfo, tupleid, &newSlot);
 	if (trigtuple == NULL)
 		return NULL;
 
@@ -2025,9 +2020,7 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
 	if (trigdesc && trigdesc->n_after_row[TRIGGER_EVENT_UPDATE] > 0)
 	{
 		HeapTuple	trigtuple = GetTupleForTrigger(estate, relinfo,
-												   tupleid,
-												   (CommandId) 0,
-												   NULL);
+												   tupleid, NULL);
 
 		AfterTriggerSaveEvent(relinfo, TRIGGER_EVENT_UPDATE,
 							  true, trigtuple, newtuple);
@@ -2038,7 +2031,7 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
 
 static HeapTuple
 GetTupleForTrigger(EState *estate, ResultRelInfo *relinfo,
-				   ItemPointer tid, CommandId cid,
+				   ItemPointer tid,
 				   TupleTableSlot **newSlot)
 {
 	Relation	relation = relinfo->ri_RelationDesc;
@@ -2060,7 +2053,8 @@ GetTupleForTrigger(EState *estate, ResultRelInfo *relinfo,
 ltrmark:;
 		tuple.t_self = *tid;
 		test = heap_lock_tuple(relation, &tuple, &buffer,
-							   &update_ctid, &update_xmax, cid,
+							   &update_ctid, &update_xmax,
+							   estate->es_output_cid,
 							   LockTupleExclusive, false);
 		switch (test)
 		{
@@ -2086,8 +2080,7 @@ ltrmark:;
 					epqslot = EvalPlanQual(estate,
 										   relinfo->ri_RangeTableIndex,
 										   &update_ctid,
-										   update_xmax,
-										   cid);
+										   update_xmax);
 					if (!TupIsNull(epqslot))
 					{
 						*tid = update_ctid;