Patchers, This patch removes the unnecesary TRANS_* states that supposedly represented "low level transaction state". The state is actually unnecesary because the states can be accurately represented using the TBLOCK_* states. This simplifies the code somewhat.
It also allows the state machinery to be actually represented as a directed graph. If you look closely you'll see that the first graph I posted yesterday was missing the StartTransactionCommand stuff; it wasn't representable in an obvious ways. This patch corrects that, by adding a new TBLOCK_STARTED state. I also attach the graph that represents this machinery. Don't add this to the source; it will probably be better to add the .dot source file. I also removed some #ifdef NOT_USED code, and the IsTransactionState() function, which can be replaced by IsTransactionOrTransactionBlock(). Maybe the latter can be renamed ... This passes the full regression test suite. Please review and apply if OK. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "I think my standards have lowered enough that now I think 'good design' is when the page doesn't irritate the living f*ck out of me." (JWZ)
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/backend/access/transam/xact.c,v
retrieving revision 1.164
diff -c -r1.164 xact.c
*** src/backend/access/transam/xact.c 11 Feb 2004 22:55:24 -0000 1.164
--- src/backend/access/transam/xact.c 27 Mar 2004 02:53:00 -0000
***************
*** 197,203 ****
FirstCommandId, /* command id */
0, /* scan command id */
0x0, /* start time */
- TRANS_DEFAULT, /* transaction state */
TBLOCK_DEFAULT /* transaction block state from the
client
* perspective */
};
--- 197,202 ----
***************
*** 239,305 ****
* ----------------------------------------------------------------
*/
- #ifdef NOT_USED
-
- /* --------------------------------
- * TransactionFlushEnabled()
- * SetTransactionFlushEnabled()
- *
- * These are used to test and set the "TransactionFlushState"
- * variable. If this variable is true (the default), then
- * the system will flush all dirty buffers to disk at the end
- * of each transaction. If false then we are assuming the
- * buffer pool resides in stable main memory, in which case we
- * only do writes as necessary.
- * --------------------------------
- */
- static int TransactionFlushState = 1;
-
- int
- TransactionFlushEnabled(void)
- {
- return TransactionFlushState;
- }
-
- void
- SetTransactionFlushEnabled(bool state)
- {
- TransactionFlushState = (state == true);
- }
- #endif
-
-
- /*
- * IsTransactionState
- *
- * This returns true if we are currently running a query
- * within an executing transaction.
- */
- 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;
- }
-
- /*
- * Shouldn't get here, but lint is not happy with this...
- */
- return false;
- }
-
/*
* IsAbortedTransactionBlockState
*
--- 238,243 ----
***************
*** 850,867 ****
TransactionState s = CurrentTransactionState;
/*
- * check the current transaction state
- */
- if (s->state != TRANS_DEFAULT)
- elog(WARNING, "StartTransaction and not in default state");
-
- /*
- * set the current transaction state information appropriately during
- * start processing
- */
- s->state = TRANS_START;
-
- /*
* Make sure we've freed any old snapshot, and reset xact state variables
*/
FreeXactSnapshot();
--- 788,793 ----
***************
*** 893,904 ****
*/
DeferredTriggerBeginXact();
- /*
- * done with start processing, set current transaction state to "in
- * progress"
- */
- s->state = TRANS_INPROGRESS;
-
}
/*
--- 819,824 ----
***************
*** 907,920 ****
static void
CommitTransaction(void)
{
- TransactionState s = CurrentTransactionState;
-
- /*
- * check the current transaction state
- */
- if (s->state != TRANS_INPROGRESS)
- elog(WARNING, "CommitTransaction and not in in-progress state");
-
/*
* Tell the trigger manager that this transaction is about to be
* committed. He'll invoke all trigger deferred until XACT before we
--- 827,832 ----
***************
*** 932,943 ****
HOLD_INTERRUPTS();
/*
- * set the current transaction state information appropriately during
- * the abort processing
- */
- s->state = TRANS_COMMIT;
-
- /*
* Do pre-commit processing (most of this stuff requires database
* access, and in fact could still cause an error...)
*/
--- 844,849 ----
***************
*** 1012,1023 ****
pgstat_count_xact_commit();
AtCommit_Memory();
- /*
- * done with commit processing, set current transaction state back to
- * default
- */
- s->state = TRANS_DEFAULT;
-
RESUME_INTERRUPTS();
}
--- 918,923 ----
***************
*** 1027,1034 ****
static void
AbortTransaction(void)
{
- TransactionState s = CurrentTransactionState;
-
/* Prevent cancel/die interrupt while cleaning up */
HOLD_INTERRUPTS();
--- 927,932 ----
***************
*** 1050,1067 ****
*/
LockWaitCancel();
- /*
- * check the current transaction state
- */
- if (s->state != TRANS_INPROGRESS)
- elog(WARNING, "AbortTransaction and not in in-progress state");
-
- /*
- * set the current transaction state information appropriately during
- * the abort processing
- */
- s->state = TRANS_ABORT;
-
/* Make sure we are in a valid memory context */
AtAbort_Memory();
--- 948,953 ----
***************
*** 1122,1130 ****
SetReindexProcessing(InvalidOid, InvalidOid);
pgstat_count_xact_rollback();
- /*
- * State remains TRANS_ABORT until CleanupTransaction().
- */
RESUME_INTERRUPTS();
}
--- 1008,1013 ----
***************
*** 1134,1158 ****
static void
CleanupTransaction(void)
{
- TransactionState s = CurrentTransactionState;
-
- /*
- * State should still be TRANS_ABORT from AbortTransaction().
- */
- if (s->state != TRANS_ABORT)
- elog(FATAL, "CleanupTransaction and not in abort state");
-
/*
* do abort cleanup processing
*/
AtCleanup_Portals(); /* now safe to release portal memory */
AtCleanup_Memory(); /* and transaction memory */
- /*
- * done with abort processing, set current transaction state back to
- * default
- */
- s->state = TRANS_DEFAULT;
}
/*
--- 1017,1028 ----
***************
*** 1171,1176 ****
--- 1041,1055 ----
*/
case TBLOCK_DEFAULT:
StartTransaction();
+ s->blockState = TBLOCK_STARTED;
+ break;
+
+ /*
+ * We should never experience this -- it means the STARTED
state
+ * was not changed in the previous CommitTransactionCommand.
+ */
+ case TBLOCK_STARTED:
+ elog(WARNING, "StartTransactionCommand: unexpected
TBLOCK_STARTED");
break;
/*
***************
*** 1247,1257 ****
switch (s->blockState)
{
/*
* If we aren't in a transaction block, just do our usual
* transaction commit.
*/
! case TBLOCK_DEFAULT:
CommitTransaction();
break;
/*
--- 1126,1146 ----
switch (s->blockState)
{
/*
+ * This shouldn't happen, because it means the previous
+ * StartTransactionCommand didn't set the STARTED state
+ * appropiately.
+ */
+ case TBLOCK_DEFAULT:
+ elog(WARNING, "CommitTransactionCommand: unexpected
TBLOCK_DEFAULT");
+ break;
+
+ /*
* If we aren't in a transaction block, just do our usual
* transaction commit.
*/
! case TBLOCK_STARTED:
CommitTransaction();
+ s->blockState = TBLOCK_DEFAULT;
break;
/*
***************
*** 1314,1324 ****
switch (s->blockState)
{
/*
* if we aren't in a transaction block, we just do the basic
* abort & cleanup transaction.
*/
! case TBLOCK_DEFAULT:
AbortTransaction();
CleanupTransaction();
break;
--- 1203,1220 ----
switch (s->blockState)
{
+ /*
+ * we aren't in a transaction, so we do nothing.
+ */
+ case TBLOCK_DEFAULT:
+ break;
+
/*
* if we aren't in a transaction block, we just do the basic
* abort & cleanup transaction.
*/
! case TBLOCK_STARTED:
! s->blockState = TBLOCK_DEFAULT;
AbortTransaction();
CleanupTransaction();
break;
***************
*** 1332,1338 ****
case TBLOCK_BEGIN:
s->blockState = TBLOCK_ABORT;
AbortTransaction();
! /* CleanupTransaction happens when we exit TBLOCK_ABORT */
break;
/*
--- 1228,1234 ----
case TBLOCK_BEGIN:
s->blockState = TBLOCK_ABORT;
AbortTransaction();
! /* CleanupTransaction happens when we exit TBLOCK_ENDABORT */
break;
/*
***************
*** 1344,1350 ****
case TBLOCK_INPROGRESS:
s->blockState = TBLOCK_ABORT;
AbortTransaction();
! /* CleanupTransaction happens when we exit TBLOCK_ABORT */
break;
/*
--- 1240,1246 ----
case TBLOCK_INPROGRESS:
s->blockState = TBLOCK_ABORT;
AbortTransaction();
! /* CleanupTransaction happens when we exit TBLOCK_ENDABORT */
break;
/*
***************
*** 1420,1426 ****
/* translator: %s represents an SQL statement name */
errmsg("%s cannot be executed from a function", stmtType)));
/* If we got past IsTransactionBlock test, should be in default state */
! if (CurrentTransactionState->blockState != TBLOCK_DEFAULT)
elog(ERROR, "cannot prevent transaction chain");
/* all okay */
}
--- 1316,1323 ----
/* translator: %s represents an SQL statement name */
errmsg("%s cannot be executed from a function", stmtType)));
/* If we got past IsTransactionBlock test, should be in default state */
! if (CurrentTransactionState->blockState != TBLOCK_DEFAULT &&
! CurrentTransactionState->blockState != TBLOCK_STARTED)
elog(ERROR, "cannot prevent transaction chain");
/* all okay */
}
***************
*** 1537,1543 ****
/*
* check the current transaction state
*/
! if (s->blockState != TBLOCK_DEFAULT)
ereport(WARNING,
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg("there is already a transaction in
progress")));
--- 1434,1440 ----
/*
* check the current transaction state
*/
! if (s->blockState != TBLOCK_STARTED)
ereport(WARNING,
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
errmsg("there is already a transaction in
progress")));
***************
*** 1551,1561 ****
/*
* do begin processing here. Nothing to do at present.
*/
-
- /*
- * done with begin processing, set block state to inprogress
- */
- s->blockState = TBLOCK_INPROGRESS;
}
/*
--- 1448,1453 ----
***************
*** 1608,1652 ****
}
/*
- * AbortTransactionBlock
- */
- #ifdef NOT_USED
- static void
- AbortTransactionBlock(void)
- {
- TransactionState s = CurrentTransactionState;
-
- /*
- * check the current transaction state
- */
- if (s->blockState == TBLOCK_INPROGRESS)
- {
- /*
- * here we were inside a transaction block something screwed up
- * inside the system so we enter the abort state, do the abort
- * processing and then return. We remain in the abort state until
- * we see an END TRANSACTION command.
- */
- s->blockState = TBLOCK_ABORT;
- AbortTransaction();
- return;
- }
-
- /*
- * here, the user issued ABORT when not inside a transaction. Issue a
- * WARNING and go to abort state. The upcoming call to
- * CommitTransactionCommand() will then put us back into the default
- * state.
- */
- ereport(WARNING,
- (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
- errmsg("there is no transaction in progress")));
- AbortTransaction();
- s->blockState = TBLOCK_ENDABORT;
- }
- #endif
-
- /*
* UserAbortTransactionBlock
*/
void
--- 1500,1505 ----
***************
*** 1669,1680 ****
{
/*
* here we were inside a transaction block and we got an abort
! * command from the user, so we move to the abort state, do the
! * abort processing and then change to the ENDABORT state so we
! * will end up in the default state after the upcoming
! * CommitTransactionCommand().
*/
- s->blockState = TBLOCK_ABORT;
AbortTransaction();
s->blockState = TBLOCK_ENDABORT;
return;
--- 1522,1531 ----
{
/*
* here we were inside a transaction block and we got an abort
! * command from the user, so we move to the ENDABORT state and
! * do abort processing so we will end up in the default state
! * after the upcoming CommitTransactionCommand().
*/
AbortTransaction();
s->blockState = TBLOCK_ENDABORT;
return;
***************
*** 1706,1733 ****
TransactionState s = CurrentTransactionState;
/*
! * Get out of any low-level transaction
*/
! switch (s->state)
{
! case TRANS_START:
! case TRANS_INPROGRESS:
! case TRANS_COMMIT:
/* In a transaction, so clean up */
AbortTransaction();
CleanupTransaction();
break;
! case TRANS_ABORT:
/* AbortTransaction already done, still need Cleanup */
CleanupTransaction();
break;
- case TRANS_DEFAULT:
- /* Not in a transaction, do nothing */
- break;
}
/*
! * Now reset the high-level state
*/
s->blockState = TBLOCK_DEFAULT;
}
--- 1557,1586 ----
TransactionState s = CurrentTransactionState;
/*
! * Get out of any transaction
*/
! switch (s->blockState)
{
! case TBLOCK_DEFAULT:
! /* Not in a transaction, do nothing */
! break;
! case TBLOCK_STARTED:
! case TBLOCK_BEGIN:
! case TBLOCK_INPROGRESS:
! case TBLOCK_END:
/* In a transaction, so clean up */
AbortTransaction();
CleanupTransaction();
break;
! case TBLOCK_ABORT:
! case TBLOCK_ENDABORT:
/* AbortTransaction already done, still need Cleanup */
CleanupTransaction();
break;
}
/*
! * Now reset the transaction state
*/
s->blockState = TBLOCK_DEFAULT;
}
***************
*** 1740,1746 ****
{
TransactionState s = CurrentTransactionState;
! if (s->blockState == TBLOCK_DEFAULT)
return false;
return true;
--- 1593,1599 ----
{
TransactionState s = CurrentTransactionState;
! if (s->blockState == TBLOCK_DEFAULT || s->blockState == TBLOCK_STARTED)
return false;
return true;
***************
*** 1758,1764 ****
{
TransactionState s = CurrentTransactionState;
! if (s->blockState == TBLOCK_DEFAULT && s->state == TRANS_DEFAULT)
return false;
return true;
--- 1611,1617 ----
{
TransactionState s = CurrentTransactionState;
! if (s->blockState == TBLOCK_DEFAULT)
return false;
return true;
***************
*** 1775,1780 ****
--- 1628,1634 ----
switch (s->blockState)
{
case TBLOCK_DEFAULT:
+ case TBLOCK_STARTED:
return 'I'; /* idle --- not in transaction
*/
case TBLOCK_BEGIN:
case TBLOCK_INPROGRESS:
Index: src/backend/catalog/namespace.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/backend/catalog/namespace.c,v
retrieving revision 1.63
diff -c -r1.63 namespace.c
*** src/backend/catalog/namespace.c 13 Feb 2004 01:08:20 -0000 1.63
--- src/backend/catalog/namespace.c 26 Mar 2004 22:52:42 -0000
***************
*** 1801,1807 ****
* If we aren't inside a transaction, we cannot do database access so
* cannot verify the individual names. Must accept the list on faith.
*/
! if (source >= PGC_S_INTERACTIVE && IsTransactionState())
{
/*
* Verify that all the names are either valid namespace names or
--- 1801,1807 ----
* If we aren't inside a transaction, we cannot do database access so
* cannot verify the individual names. Must accept the list on faith.
*/
! if (source >= PGC_S_INTERACTIVE && IsTransactionOrTransactionBlock())
{
/*
* Verify that all the names are either valid namespace names or
Index: src/backend/commands/variable.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/backend/commands/variable.c,v
retrieving revision 1.93
diff -c -r1.93 variable.c
*** src/backend/commands/variable.c 19 Jan 2004 19:04:40 -0000 1.93
--- src/backend/commands/variable.c 26 Mar 2004 22:53:02 -0000
***************
*** 776,782 ****
/* not a saved ID, so look it up */
HeapTuple userTup;
! if (!IsTransactionState())
{
/*
* Can't do catalog lookups, so fail. The upshot of this is
--- 776,782 ----
/* not a saved ID, so look it up */
HeapTuple userTup;
! if (!IsTransactionOrTransactionBlock())
{
/*
* Can't do catalog lookups, so fail. The upshot of this is
Index: src/backend/utils/mb/mbutils.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/backend/utils/mb/mbutils.c,v
retrieving revision 1.46
diff -c -r1.46 mbutils.c
*** src/backend/utils/mb/mbutils.c 15 Mar 2004 10:41:25 -0000 1.46
--- src/backend/utils/mb/mbutils.c 26 Mar 2004 22:53:33 -0000
***************
*** 114,120 ****
* postgresql.conf. Which would probably be a stupid thing to do
* anyway.
*/
! if (!IsTransactionState())
return -1;
/*
--- 114,120 ----
* postgresql.conf. Which would probably be a stupid thing to do
* anyway.
*/
! if (!IsTransactionOrTransactionBlock())
return -1;
/*
***************
*** 232,238 ****
unsigned char *result;
Oid proc;
! if (!IsTransactionState())
return src;
if (src_encoding == dest_encoding)
--- 232,238 ----
unsigned char *result;
Oid proc;
! if (!IsTransactionOrTransactionBlock())
return src;
if (src_encoding == dest_encoding)
Index: src/include/access/xact.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql-server/src/include/access/xact.h,v
retrieving revision 1.61
diff -c -r1.61 xact.h
*** src/include/access/xact.h 11 Feb 2004 22:55:25 -0000 1.61
--- src/include/access/xact.h 27 Mar 2004 02:52:30 -0000
***************
*** 41,63 ****
extern bool XactReadOnly;
/*
- * transaction states - transaction state from server perspective
- */
- typedef enum TransState
- {
- TRANS_DEFAULT,
- TRANS_START,
- TRANS_INPROGRESS,
- TRANS_COMMIT,
- TRANS_ABORT
- } TransState;
-
- /*
* transaction block states - transaction state of client queries
*/
typedef enum TBlockState
{
TBLOCK_DEFAULT,
TBLOCK_BEGIN,
TBLOCK_INPROGRESS,
TBLOCK_END,
--- 41,52 ----
extern bool XactReadOnly;
/*
* transaction block states - transaction state of client queries
*/
typedef enum TBlockState
{
TBLOCK_DEFAULT,
+ TBLOCK_STARTED,
TBLOCK_BEGIN,
TBLOCK_INPROGRESS,
TBLOCK_END,
***************
*** 79,85 ****
CommandId commandId;
AbsoluteTime startTime;
int startTimeUsec;
- TransState state;
TBlockState blockState;
} TransactionStateData;
--- 68,73 ----
***************
*** 123,129 ****
* extern definitions
* ----------------
*/
- extern bool IsTransactionState(void);
extern bool IsAbortedTransactionBlockState(void);
extern TransactionId GetCurrentTransactionId(void);
extern CommandId GetCurrentCommandId(void);
--- 111,116 ----
<<attachment: add_starting.png>>
---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faqs/FAQ.html
