Redefine IsTransactionState() to only return true for TRANS_INPROGRESS state,

tglsfdc · tglsfdc · commit 6d6d14b6d52f · 2007-06-07T21:45:59.000Z
which is the only state in which it's safe to initiate database queries.
It turns out that all but two of the callers thought that's what it meant;
and the other two were using it as a proxy for "will GetTopTransactionId()
return a nonzero XID"?  Since it was in fact an unreliable guide to that,
make those two just invoke GetTopTransactionId() always, then deal with a
zero result if they get one.
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.244 2007/05/30 21:01:39 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/transam/xact.c,v 1.245 2007/06/07 21:45:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -64,12 +64,12 @@ int			CommitSiblings = 5; /* # concurrent xacts needed to sleep */
  */
 typedef enum TransState
 {
-	TRANS_DEFAULT,
-	TRANS_START,
-	TRANS_INPROGRESS,
-	TRANS_COMMIT,
-	TRANS_ABORT,
-	TRANS_PREPARE
+	TRANS_DEFAULT,				/* idle */
+	TRANS_START,				/* transaction starting */
+	TRANS_INPROGRESS,			/* inside a valid transaction */
+	TRANS_COMMIT,				/* commit in progress */
+	TRANS_ABORT,				/* abort in progress */
+	TRANS_PREPARE				/* prepare in progress */
 } TransState;
 
 /*
@@ -255,34 +255,22 @@ static const char *TransStateAsString(TransState state);
 /*
  *	IsTransactionState
  *
- *	This returns true if we are currently running a query
- *	within an executing transaction.
+ *	This returns true if we are inside a valid transaction; that is,
+ *	it is safe to initiate database access, take heavyweight locks, etc.
  */
 bool
 IsTransactionState(void)
 {
 	TransactionState s = CurrentTransactionState;
 
-	switch (s->state)
-	{
-		case TRANS_DEFAULT:
-			return false;
-		case TRANS_START:
-			return true;
-		case TRANS_INPROGRESS:
-			return true;
-		case TRANS_COMMIT:
-			return true;
-		case TRANS_ABORT:
-			return true;
-		case TRANS_PREPARE:
-			return true;
-	}
-
 	/*
-	 * Shouldn't get here, but lint is not happy without this...
+	 * TRANS_DEFAULT and TRANS_ABORT are obviously unsafe states.  However,
+	 * we also reject the startup/shutdown states TRANS_START, TRANS_COMMIT,
+	 * TRANS_PREPARE since it might be too soon or too late within those
+	 * transition states to do anything interesting.  Hence, the only "valid"
+	 * state is TRANS_INPROGRESS.
 	 */
-	return false;
+	return (s->state == TRANS_INPROGRESS);
 }
 
 /*
@@ -308,7 +296,9 @@ IsAbortedTransactionBlockState(void)
  *	GetTopTransactionId
  *
  * Get the ID of the main transaction, even if we are currently inside
- * a subtransaction.
+ * a subtransaction.  If we are not in a transaction at all, or if we
+ * are in transaction startup and haven't yet assigned an XID,
+ * InvalidTransactionId is returned.
  */
 TransactionId
 GetTopTransactionId(void)
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
@@ -23,7 +23,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.25 2007/06/01 19:38:07 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.26 2007/06/07 21:45:59 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -422,9 +422,8 @@ GetOldestXmin(bool allDbs, bool ignoreVacuum)
 	 * are no xacts running at all, that will be the subtrans truncation
 	 * point!)
 	 */
-	if (IsTransactionState())
-		result = GetTopTransactionId();
-	else
+	result = GetTopTransactionId();
+	if (!TransactionIdIsValid(result))
 		result = ReadNewTransactionId();
 
 	LWLockAcquire(ProcArrayLock, LW_SHARED);
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
@@ -42,7 +42,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.185 2007/05/04 02:01:02 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.186 2007/06/07 21:45:59 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1593,12 +1593,7 @@ log_line_prefix(StringInfo buf)
 				break;
 			case 'x':
 				if (MyProcPort)
-				{
-					if (IsTransactionState())
-						appendStringInfo(buf, "%u", GetTopTransactionId());
-					else
-						appendStringInfo(buf, "%u", InvalidTransactionId);
-				}
+					appendStringInfo(buf, "%u", GetTopTransactionId());
 				break;
 			case '%':
 				appendStringInfoChar(buf, '%');

Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@`
`23`	`23`	`*`
`24`	`24`	`*`
`25`	`25`	`* IDENTIFICATION`
`26`		`- * $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.25 2007/06/01 19:38:07 tgl Exp $`
	`26`	`+ * $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.26 2007/06/07 21:45:59 tgl Exp $`
`27`	`27`	`*`
`28`	`28`	`*-------------------------------------------------------------------------`
`29`	`29`	`*/`
`@@ -422,9 +422,8 @@ GetOldestXmin(bool allDbs, bool ignoreVacuum)`
`422`	`422`	`* are no xacts running at all, that will be the subtrans truncation`
`423`	`423`	`* point!)`
`424`	`424`	`*/`
`425`		`- if (IsTransactionState())`
`426`		`- result = GetTopTransactionId();`
`427`		`- else`
	`425`	`+ result = GetTopTransactionId();`
	`426`	`+ if (!TransactionIdIsValid(result))`
`428`	`427`	`result = ReadNewTransactionId();`
`429`	`428`
`430`	`429`	`LWLockAcquire(ProcArrayLock, LW_SHARED);`