diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 5e7e812200..5880690c88 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -145,6 +145,7 @@ typedef enum TBlockState /* transaction block states */ TBLOCK_BEGIN, /* starting transaction block */ TBLOCK_INPROGRESS, /* live transaction */ + TBLOCK_IMPLICIT_INPROGRESS, /* live transaction after implicit BEGIN */ TBLOCK_PARALLEL_INPROGRESS, /* live transaction inside parallel worker */ TBLOCK_END, /* COMMIT received */ TBLOCK_ABORT, /* failed xact, awaiting ROLLBACK */ @@ -2700,6 +2701,7 @@ StartTransactionCommand(void) * previous CommitTransactionCommand.) */ case TBLOCK_INPROGRESS: + case TBLOCK_IMPLICIT_INPROGRESS: case TBLOCK_SUBINPROGRESS: break; @@ -2790,6 +2792,7 @@ CommitTransactionCommand(void) * counter and return. */ case TBLOCK_INPROGRESS: + case TBLOCK_IMPLICIT_INPROGRESS: case TBLOCK_SUBINPROGRESS: CommandCounterIncrement(); break; @@ -2867,7 +2870,8 @@ CommitTransactionCommand(void) } while (s->blockState == TBLOCK_SUBRELEASE); Assert(s->blockState == TBLOCK_INPROGRESS || - s->blockState == TBLOCK_SUBINPROGRESS); + s->blockState == TBLOCK_SUBINPROGRESS || + s->blockState == TBLOCK_IMPLICIT_INPROGRESS); break; /* @@ -3014,10 +3018,12 @@ AbortCurrentTransaction(void) break; /* - * if we aren't in a transaction block, we just do the basic abort - * & cleanup transaction. + * If we aren't in a transaction block, we just do the basic abort + * & cleanup transaction. We treat an implicit transaction block + * similarly. */ case TBLOCK_STARTED: + case TBLOCK_IMPLICIT_INPROGRESS: AbortTransaction(); CleanupTransaction(); s->blockState = TBLOCK_DEFAULT; @@ -3148,9 +3154,8 @@ AbortCurrentTransaction(void) * completes). Subtransactions are verboten too. * * isTopLevel: passed down from ProcessUtility to determine whether we are - * inside a function or multi-query querystring. (We will always fail if - * this is false, but it's convenient to centralize the check here instead of - * making callers do it.) + * inside a function. (We will always fail if this is false, but it's + * convenient to centralize the check here instead of making callers do it.) * stmtType: statement type name, for error messages. */ void @@ -3183,8 +3188,7 @@ PreventTransactionChain(bool isTopLevel, const char *stmtType) ereport(ERROR, (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION), /* translator: %s represents an SQL statement name */ - errmsg("%s cannot be executed from a function or multi-command string", - stmtType))); + 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 && @@ -3428,6 +3432,15 @@ BeginTransactionBlock(void) s->blockState = TBLOCK_BEGIN; break; + /* + * BEGIN converts an implicit transaction block to a regular one. + * (Note that we allow this even if we've already done some + * commands, which is a bit odd but matches historical practice.) + */ + case TBLOCK_IMPLICIT_INPROGRESS: + s->blockState = TBLOCK_BEGIN; + break; + /* * Already a transaction block in progress. */ @@ -3503,7 +3516,8 @@ PrepareTransactionBlock(char *gid) * ignore case where we are not in a transaction; * EndTransactionBlock already issued a warning. */ - Assert(s->blockState == TBLOCK_STARTED); + Assert(s->blockState == TBLOCK_STARTED || + s->blockState == TBLOCK_IMPLICIT_INPROGRESS); /* Don't send back a PREPARE result tag... */ result = false; } @@ -3541,6 +3555,18 @@ EndTransactionBlock(void) result = true; break; + /* + * In an implicit transaction block, commit, but issue a warning + * because there was no explicit BEGIN before this. + */ + case TBLOCK_IMPLICIT_INPROGRESS: + ereport(WARNING, + (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION), + errmsg("there is no transaction in progress"))); + s->blockState = TBLOCK_END; + result = true; + break; + /* * We are in a failed transaction block. Tell * CommitTransactionCommand it's time to exit the block. @@ -3556,14 +3582,16 @@ EndTransactionBlock(void) case TBLOCK_SUBINPROGRESS: while (s->parent != NULL) { - if (s->blockState == TBLOCK_SUBINPROGRESS) + if (s->blockState == TBLOCK_SUBINPROGRESS || + s->blockState == TBLOCK_IMPLICIT_INPROGRESS) s->blockState = TBLOCK_SUBCOMMIT; else elog(FATAL, "EndTransactionBlock: unexpected state %s", BlockStateAsString(s->blockState)); s = s->parent; } - if (s->blockState == TBLOCK_INPROGRESS) + if (s->blockState == TBLOCK_INPROGRESS || + s->blockState == TBLOCK_IMPLICIT_INPROGRESS) s->blockState = TBLOCK_END; else elog(FATAL, "EndTransactionBlock: unexpected state %s", @@ -3682,7 +3710,8 @@ UserAbortTransactionBlock(void) case TBLOCK_SUBABORT: while (s->parent != NULL) { - if (s->blockState == TBLOCK_SUBINPROGRESS) + if (s->blockState == TBLOCK_SUBINPROGRESS || + s->blockState == TBLOCK_IMPLICIT_INPROGRESS) s->blockState = TBLOCK_SUBABORT_PENDING; else if (s->blockState == TBLOCK_SUBABORT) s->blockState = TBLOCK_SUBABORT_END; @@ -3691,7 +3720,8 @@ UserAbortTransactionBlock(void) BlockStateAsString(s->blockState)); s = s->parent; } - if (s->blockState == TBLOCK_INPROGRESS) + if (s->blockState == TBLOCK_INPROGRESS || + s->blockState == TBLOCK_IMPLICIT_INPROGRESS) s->blockState = TBLOCK_ABORT_PENDING; else if (s->blockState == TBLOCK_ABORT) s->blockState = TBLOCK_ABORT_END; @@ -3705,8 +3735,14 @@ UserAbortTransactionBlock(void) * WARNING and go to abort state. The upcoming call to * CommitTransactionCommand() will then put us back into the * default state. + * + * We do the same thing with ABORT inside an implicit transaction, + * although in this case we might be rolling back actual database + * state changes. (It's debatable whether we should issue a + * WARNING in this case, but we have done so historically.) */ case TBLOCK_STARTED: + case TBLOCK_IMPLICIT_INPROGRESS: ereport(WARNING, (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION), errmsg("there is no transaction in progress"))); @@ -3743,6 +3779,58 @@ UserAbortTransactionBlock(void) } } +/* + * BeginImplicitTransactionBlock + * Start an implicit transaction block if we're not already in one. + * + * Unlike BeginTransactionBlock, this is called directly from the main loop + * in postgres.c, not within a Portal. So we can just change blockState + * without a lot of ceremony. We do not expect caller to do + * CommitTransactionCommand/StartTransactionCommand. + */ +void +BeginImplicitTransactionBlock(void) +{ + TransactionState s = CurrentTransactionState; + + /* + * If we are in STARTED state (that is, no transaction block is open), + * switch to IMPLICIT_INPROGRESS state, creating an implicit transaction + * block. + * + * For caller convenience, we consider all other transaction states as + * legal here; otherwise the caller would need its own state check, which + * seems rather pointless. + */ + if (s->blockState == TBLOCK_STARTED) + s->blockState = TBLOCK_IMPLICIT_INPROGRESS; +} + +/* + * EndImplicitTransactionBlock + * End an implicit transaction block, if we're in one. + * + * Like EndTransactionBlock, we just make any needed blockState change here. + * The real work will be done in the upcoming CommitTransactionCommand(). + */ +void +EndImplicitTransactionBlock(void) +{ + TransactionState s = CurrentTransactionState; + + /* + * If we are in IMPLICIT_INPROGRESS state, switch back to STARTED state, + * allowing CommitTransactionCommand to commit whatever happened during + * the implicit transaction block as though it were a single statement. + * + * For caller convenience, we consider all other transaction states as + * legal here; otherwise the caller would need its own state check, which + * seems rather pointless. + */ + if (s->blockState == TBLOCK_IMPLICIT_INPROGRESS) + s->blockState = TBLOCK_STARTED; +} + /* * DefineSavepoint * This executes a SAVEPOINT command. @@ -3768,6 +3856,7 @@ DefineSavepoint(char *name) { case TBLOCK_INPROGRESS: case TBLOCK_SUBINPROGRESS: + case TBLOCK_IMPLICIT_INPROGRESS: /* Normal subtransaction start */ PushTransaction(); s = CurrentTransactionState; /* changed by push */ @@ -3834,8 +3923,7 @@ ReleaseSavepoint(List *options) switch (s->blockState) { /* - * We can't rollback to a savepoint if there is no savepoint - * defined. + * We can't release a savepoint if there is no savepoint defined. */ case TBLOCK_INPROGRESS: ereport(ERROR, @@ -3852,6 +3940,7 @@ ReleaseSavepoint(List *options) /* These cases are invalid. */ case TBLOCK_DEFAULT: + case TBLOCK_IMPLICIT_INPROGRESS: case TBLOCK_STARTED: case TBLOCK_BEGIN: case TBLOCK_PARALLEL_INPROGRESS: @@ -3966,6 +4055,7 @@ RollbackToSavepoint(List *options) /* These cases are invalid. */ case TBLOCK_DEFAULT: + case TBLOCK_IMPLICIT_INPROGRESS: case TBLOCK_STARTED: case TBLOCK_BEGIN: case TBLOCK_PARALLEL_INPROGRESS: @@ -4046,11 +4136,12 @@ RollbackToSavepoint(List *options) /* * BeginInternalSubTransaction * This is the same as DefineSavepoint except it allows TBLOCK_STARTED, - * TBLOCK_END, and TBLOCK_PREPARE states, and therefore it can safely be - * used in functions that might be called when not inside a BEGIN block - * or when running deferred triggers at COMMIT/PREPARE time. Also, it - * automatically does CommitTransactionCommand/StartTransactionCommand - * instead of expecting the caller to do it. + * TBLOCK_IMPLICIT_INPROGRESS, TBLOCK_END, and TBLOCK_PREPARE states, + * and therefore it can safely be used in functions that might be called + * when not inside a BEGIN block or when running deferred triggers at + * COMMIT/PREPARE time. Also, it automatically does + * CommitTransactionCommand/StartTransactionCommand instead of expecting + * the caller to do it. */ void BeginInternalSubTransaction(char *name) @@ -4076,6 +4167,7 @@ BeginInternalSubTransaction(char *name) { case TBLOCK_STARTED: case TBLOCK_INPROGRESS: + case TBLOCK_IMPLICIT_INPROGRESS: case TBLOCK_END: case TBLOCK_PREPARE: case TBLOCK_SUBINPROGRESS: @@ -4180,6 +4272,7 @@ RollbackAndReleaseCurrentSubTransaction(void) case TBLOCK_DEFAULT: case TBLOCK_STARTED: case TBLOCK_BEGIN: + case TBLOCK_IMPLICIT_INPROGRESS: case TBLOCK_PARALLEL_INPROGRESS: case TBLOCK_SUBBEGIN: case TBLOCK_INPROGRESS: @@ -4211,6 +4304,7 @@ RollbackAndReleaseCurrentSubTransaction(void) s = CurrentTransactionState; /* changed by pop */ AssertState(s->blockState == TBLOCK_SUBINPROGRESS || s->blockState == TBLOCK_INPROGRESS || + s->blockState == TBLOCK_IMPLICIT_INPROGRESS || s->blockState == TBLOCK_STARTED); } @@ -4259,6 +4353,7 @@ AbortOutOfAnyTransaction(void) case TBLOCK_STARTED: case TBLOCK_BEGIN: case TBLOCK_INPROGRESS: + case TBLOCK_IMPLICIT_INPROGRESS: case TBLOCK_PARALLEL_INPROGRESS: case TBLOCK_END: case TBLOCK_ABORT_PENDING: @@ -4369,6 +4464,7 @@ TransactionBlockStatusCode(void) case TBLOCK_BEGIN: case TBLOCK_SUBBEGIN: case TBLOCK_INPROGRESS: + case TBLOCK_IMPLICIT_INPROGRESS: case TBLOCK_PARALLEL_INPROGRESS: case TBLOCK_SUBINPROGRESS: case TBLOCK_END: @@ -5036,6 +5132,8 @@ BlockStateAsString(TBlockState blockState) return "BEGIN"; case TBLOCK_INPROGRESS: return "INPROGRESS"; + case TBLOCK_IMPLICIT_INPROGRESS: + return "IMPLICIT_INPROGRESS"; case TBLOCK_PARALLEL_INPROGRESS: return "PARALLEL_INPROGRESS"; case TBLOCK_END: diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 8d3fecf6d6..c10d891260 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -883,10 +883,9 @@ exec_simple_query(const char *query_string) ListCell *parsetree_item; bool save_log_statement_stats = log_statement_stats; bool was_logged = false; - bool isTopLevel; + bool use_implicit_block; char msec_str[32]; - /* * Report query to various monitoring facilities. */ @@ -947,13 +946,14 @@ exec_simple_query(const char *query_string) MemoryContextSwitchTo(oldcontext); /* - * We'll tell PortalRun it's a top-level command iff there's exactly one - * raw parsetree. If more than one, it's effectively a transaction block - * and we want PreventTransactionChain to reject unsafe commands. (Note: - * we're assuming that query rewrite cannot add commands that are - * significant to PreventTransactionChain.) + * For historical reasons, if multiple SQL statements are given in a + * single "simple Query" message, we execute them as a single transaction, + * unless explicit transaction control commands are included to make + * portions of the list be separate transactions. To represent this + * behavior properly in the transaction machinery, we use an "implicit" + * transaction block. */ - isTopLevel = (list_length(parsetree_list) == 1); + use_implicit_block = (list_length(parsetree_list) > 1); /* * Run through the raw parsetree(s) and process each one. @@ -1001,6 +1001,16 @@ exec_simple_query(const char *query_string) /* Make sure we are in a transaction command */ start_xact_command(); + /* + * If using an implicit transaction block, and we're not already in a + * transaction block, start an implicit block to force this statement + * to be grouped together with any following ones. (We must do this + * each time through the loop; otherwise, a COMMIT/ROLLBACK in the + * list would cause later statements to not be grouped.) + */ + if (use_implicit_block) + BeginImplicitTransactionBlock(); + /* If we got a cancel signal in parsing or prior command, quit */ CHECK_FOR_INTERRUPTS(); @@ -1098,7 +1108,7 @@ exec_simple_query(const char *query_string) */ (void) PortalRun(portal, FETCH_ALL, - isTopLevel, + true, /* always top level */ true, receiver, receiver, @@ -1108,15 +1118,7 @@ exec_simple_query(const char *query_string) PortalDrop(portal, false); - if (IsA(parsetree->stmt, TransactionStmt)) - { - /* - * If this was a transaction control statement, commit it. We will - * start a new xact command for the next command (if any). - */ - finish_xact_command(); - } - else if (lnext(parsetree_item) == NULL) + if (lnext(parsetree_item) == NULL) { /* * If this is the last parsetree of the query string, close down @@ -1124,9 +1126,18 @@ exec_simple_query(const char *query_string) * is so that any end-of-transaction errors are reported before * the command-complete message is issued, to avoid confusing * clients who will expect either a command-complete message or an - * error, not one and then the other. But for compatibility with - * historical Postgres behavior, we do not force a transaction - * boundary between queries appearing in a single query string. + * error, not one and then the other. Also, if we're using an + * implicit transaction block, we must close that out first. + */ + if (use_implicit_block) + EndImplicitTransactionBlock(); + finish_xact_command(); + } + else if (IsA(parsetree->stmt, TransactionStmt)) + { + /* + * If this was a transaction control statement, commit it. We will + * start a new xact command for the next command. */ finish_xact_command(); } @@ -1149,7 +1160,9 @@ exec_simple_query(const char *query_string) } /* end loop over parsetrees */ /* - * Close down transaction statement, if one is open. + * Close down transaction statement, if one is open. (This will only do + * something if the parsetree list was empty; otherwise the last loop + * iteration already did it.) */ finish_xact_command(); diff --git a/src/include/access/xact.h b/src/include/access/xact.h index ad5aad96df..f2c10f905f 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -352,6 +352,8 @@ extern void BeginTransactionBlock(void); extern bool EndTransactionBlock(void); extern bool PrepareTransactionBlock(char *gid); extern void UserAbortTransactionBlock(void); +extern void BeginImplicitTransactionBlock(void); +extern void EndImplicitTransactionBlock(void); extern void ReleaseSavepoint(List *options); extern void DefineSavepoint(char *name); extern void RollbackToSavepoint(List *options); diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out index d9b702d016..c3b7e846ca 100644 --- a/src/test/regress/expected/transactions.out +++ b/src/test/regress/expected/transactions.out @@ -659,6 +659,95 @@ ERROR: portal "ctt" cannot be run COMMIT; DROP FUNCTION create_temp_tab(); DROP FUNCTION invert(x float8); +-- Test assorted behaviors around the implicit transaction block created +-- when multiple SQL commands are sent in a single Query message. These +-- tests rely on the fact that psql will not break SQL commands apart at a +-- backslash-quoted semicolon, but will send them as one Query. +create temp table i_table (f1 int); +-- psql will show only the last result in a multi-statement Query +SELECT 1\; SELECT 2\; SELECT 3; + ?column? +---------- + 3 +(1 row) + +-- this implicitly commits: +insert into i_table values(1)\; select * from i_table; + f1 +---- + 1 +(1 row) + +-- 1/0 error will cause rolling back the whole implicit transaction +insert into i_table values(2)\; select * from i_table\; select 1/0; +ERROR: division by zero +select * from i_table; + f1 +---- + 1 +(1 row) + +rollback; -- we are not in a transaction at this point +WARNING: there is no transaction in progress +-- can use regular begin/commit/rollback within a single Query +begin\; insert into i_table values(3)\; commit; +rollback; -- we are not in a transaction at this point +WARNING: there is no transaction in progress +begin\; insert into i_table values(4)\; rollback; +rollback; -- we are not in a transaction at this point +WARNING: there is no transaction in progress +-- begin converts implicit transaction into a regular one that +-- can extend past the end of the Query +select 1\; begin\; insert into i_table values(5); +commit; +select 1\; begin\; insert into i_table values(6); +rollback; +-- commit in implicit-transaction state commits but issues a warning. +insert into i_table values(7)\; commit\; insert into i_table values(8)\; select 1/0; +WARNING: there is no transaction in progress +ERROR: division by zero +-- similarly, rollback aborts but issues a warning. +insert into i_table values(9)\; rollback\; select 2; +WARNING: there is no transaction in progress + ?column? +---------- + 2 +(1 row) + +select * from i_table; + f1 +---- + 1 + 3 + 5 + 7 +(4 rows) + +rollback; -- we are not in a transaction at this point +WARNING: there is no transaction in progress +-- implicit transaction block is still a transaction block, for e.g. VACUUM +SELECT 1\; VACUUM; +ERROR: VACUUM cannot run inside a transaction block +SELECT 1\; COMMIT\; VACUUM; +WARNING: there is no transaction in progress +ERROR: VACUUM cannot run inside a transaction block +-- we allow savepoint-related commands in implicit-transaction state +SELECT 1\; SAVEPOINT sp; +SELECT 1\; COMMIT\; SAVEPOINT sp; +ROLLBACK TO SAVEPOINT sp\; SELECT 2; + ?column? +---------- + 2 +(1 row) + +SELECT 2\; RELEASE SAVEPOINT sp\; SELECT 3; + ?column? +---------- + 3 +(1 row) + +-- but this is OK, because the BEGIN converts it to a regular xact +SELECT 1\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT; -- Test for successful cleanup of an aborted transaction at session exit. -- THIS MUST BE THE LAST TEST IN THIS FILE. begin; diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql index bf9cb05971..483cf7e5f0 100644 --- a/src/test/regress/sql/transactions.sql +++ b/src/test/regress/sql/transactions.sql @@ -419,6 +419,60 @@ DROP FUNCTION create_temp_tab(); DROP FUNCTION invert(x float8); +-- Test assorted behaviors around the implicit transaction block created +-- when multiple SQL commands are sent in a single Query message. These +-- tests rely on the fact that psql will not break SQL commands apart at a +-- backslash-quoted semicolon, but will send them as one Query. + +create temp table i_table (f1 int); + +-- psql will show only the last result in a multi-statement Query +SELECT 1\; SELECT 2\; SELECT 3; + +-- this implicitly commits: +insert into i_table values(1)\; select * from i_table; +-- 1/0 error will cause rolling back the whole implicit transaction +insert into i_table values(2)\; select * from i_table\; select 1/0; +select * from i_table; + +rollback; -- we are not in a transaction at this point + +-- can use regular begin/commit/rollback within a single Query +begin\; insert into i_table values(3)\; commit; +rollback; -- we are not in a transaction at this point +begin\; insert into i_table values(4)\; rollback; +rollback; -- we are not in a transaction at this point + +-- begin converts implicit transaction into a regular one that +-- can extend past the end of the Query +select 1\; begin\; insert into i_table values(5); +commit; +select 1\; begin\; insert into i_table values(6); +rollback; + +-- commit in implicit-transaction state commits but issues a warning. +insert into i_table values(7)\; commit\; insert into i_table values(8)\; select 1/0; +-- similarly, rollback aborts but issues a warning. +insert into i_table values(9)\; rollback\; select 2; + +select * from i_table; + +rollback; -- we are not in a transaction at this point + +-- implicit transaction block is still a transaction block, for e.g. VACUUM +SELECT 1\; VACUUM; +SELECT 1\; COMMIT\; VACUUM; + +-- we allow savepoint-related commands in implicit-transaction state +SELECT 1\; SAVEPOINT sp; +SELECT 1\; COMMIT\; SAVEPOINT sp; +ROLLBACK TO SAVEPOINT sp\; SELECT 2; +SELECT 2\; RELEASE SAVEPOINT sp\; SELECT 3; + +-- but this is OK, because the BEGIN converts it to a regular xact +SELECT 1\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT; + + -- Test for successful cleanup of an aborted transaction at session exit. -- THIS MUST BE THE LAST TEST IN THIS FILE.