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 000c72f8de1933a8a54d74cbe998be9a91773198..72a7cf40a637d970e6cdb88e3f5c2ce7b970b099 100644 (file)
--- 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
index 287d3b4ee56162c1a766477bba2e92d0529fad9d..9d290ba97393460fd2416e132e71b33b3349b69e 100644 (file)
--- 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
index 93e7663da3aca50e02bedbf9c0b5424171edcbb2..c6952ef20e85e45e6fc266b436ffba025d4673e2 100644 (file)
--- 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, '%');