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

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
index b2f300a77073feac225a72ceb012d0cde8041429..9dc643bca4426a020b6d8dc0036d78c4749acb24 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -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
index 4813b6ec2dfe9d6ec79cc004c87f870063dfa56d..5361c88205cce35cce8808023fb30c735d26bf28 100644 (file)
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -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
index 58feb2b632612713bef759199301e2bf783902db..88a295d20f85ec2d2d03a8743977f0b3b60b3b4d 100644 (file)
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -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, '%');