I would like to bring up again the topic of statement-level rollback. This was discussed in some depth at [1]. This patch is not based on Tsunakawa-san's patch submitted in that thread; although I started from it, I eventually removed almost everything and replaced with a completely different implementation.
Patch 0001 here attached introduces the new setting and behavior with a few applicability restrictions I outlined in [2], on safety grounds. However, that wasn't exactly met with the standing ovation that I was expecting[3], so patch 0002 removes those and changes the behavior to that of any other GUC -- so much so, in fact, that after that patch you can change the rollback scope in the middle of a transaction, and it affects from that point onwards. There is a definite user demand for this feature; almost anyone porting nontrivial apps from other DBMS systems will want this to be an option. Of course, the default behavior doesn't change. Some commercial DBMSs offer only the behavior that this patch introduces. While they aren't universally loved, they have lots of users, and many of those users have migrated or are migrating or will migrate to Postgres in some form or another. Adding this feature lowers the barrier to such migrations, which cannot be but a good thing. I think we should add this, and if that means some tools will need to add a SET line to ensure they get the behavior they need in case careless DBAs enable this behavior server-wide, that seems not a terribly onerous cost anyway. [1] https://postgr.es/m/0A3221C70F24FB45833433255569204D1F6A9286@G01JPEXMBYT05 [2] https://postgr.es/m/20180615202328.7m46qo46v5a5wkd2@alvherre.pgsql [3] https://postgr.es/m/CA+TgmoavJybY0C8LXHZNcw4p=Eh48yoZwbjq8HVa10qhuP=g...@mail.gmail.com -- Álvaro Herrera PostgreSQL Expert, https://www.2ndQuadrant.com/
>From 5201b4686c47ac37f53debe4ed3851d4ebc224b6 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Fri, 7 Dec 2018 15:50:45 -0300 Subject: [PATCH 1/2] Support statement-level rollback --- doc/src/sgml/ref/begin.sgml | 5 +- doc/src/sgml/ref/set_transaction.sgml | 22 +++++- src/backend/access/transam/xact.c | 120 ++++++++++++++++++++++++++--- src/backend/commands/variable.c | 31 ++++++++ src/backend/parser/gram.y | 12 ++- src/backend/tcop/utility.c | 4 + src/backend/utils/misc/guc.c | 20 +++++ src/include/access/xact.h | 11 +++ src/include/commands/variable.h | 1 + src/include/parser/kwlist.h | 1 + src/test/regress/expected/transactions.out | 70 +++++++++++++++++ src/test/regress/sql/transactions.sql | 46 +++++++++++ 12 files changed, 328 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/ref/begin.sgml b/doc/src/sgml/ref/begin.sgml index c23bbfb4e7..a60a3abae3 100644 --- a/doc/src/sgml/ref/begin.sgml +++ b/doc/src/sgml/ref/begin.sgml @@ -26,6 +26,7 @@ BEGIN [ WORK | TRANSACTION ] [ <replaceable class="parameter">transaction_mode</ <phrase>where <replaceable class="parameter">transaction_mode</replaceable> is one of:</phrase> ISOLATION LEVEL { SERIALIZABLE | REPEATABLE READ | READ COMMITTED | READ UNCOMMITTED } + ROLLBACK SCOPE TRANSACTION READ WRITE | READ ONLY [ NOT ] DEFERRABLE </synopsis> @@ -58,8 +59,8 @@ BEGIN [ WORK | TRANSACTION ] [ <replaceable class="parameter">transaction_mode</ </para> <para> - If the isolation level, read/write mode, or deferrable mode is specified, the new - transaction has those characteristics, as if + If the isolation level, read/write mode, rollback scope, or deferrable mode + is specified, the new transaction has those characteristics, as if <xref linkend="sql-set-transaction"/> was executed. </para> diff --git a/doc/src/sgml/ref/set_transaction.sgml b/doc/src/sgml/ref/set_transaction.sgml index 43b1c6c892..e6aa4f46f6 100644 --- a/doc/src/sgml/ref/set_transaction.sgml +++ b/doc/src/sgml/ref/set_transaction.sgml @@ -10,6 +10,11 @@ </indexterm> <indexterm> + <primary>transaction rollback scope</primary> + <secondary>setting</secondary> + </indexterm> + + <indexterm> <primary>read-only transaction</primary> <secondary>setting</secondary> </indexterm> @@ -39,6 +44,7 @@ SET SESSION CHARACTERISTICS AS TRANSACTION <replaceable class="parameter">transa <phrase>where <replaceable class="parameter">transaction_mode</replaceable> is one of:</phrase> ISOLATION LEVEL { SERIALIZABLE | REPEATABLE READ | READ COMMITTED | READ UNCOMMITTED } + ROLLBACK SCOPE TRANSACTION READ WRITE | READ ONLY [ NOT ] DEFERRABLE </synopsis> @@ -59,7 +65,8 @@ SET SESSION CHARACTERISTICS AS TRANSACTION <replaceable class="parameter">transa <para> The available transaction characteristics are the transaction - isolation level, the transaction access mode (read/write or + isolation level, the rollback scope, + the transaction access mode (read/write or read-only), and the deferrable mode. In addition, a snapshot can be selected, though only for the current transaction, not as a session default. @@ -124,6 +131,19 @@ SET SESSION CHARACTERISTICS AS TRANSACTION <replaceable class="parameter">transa </para> <para> + The rollback scope of a transaction determines the range of operations + which roll back when an SQL statement fails. The default value is + <literal>TRANSACTION</literal>, which causes the entire transaction or + current subtransaction to be rolled back. This is the only mode that can + be selected with <command>SET TRANSACTION</command>. The other possible + mode, <literal>STATEMENT</literal>, can only be specified during connection + establishment, <literal>ALTER USER</literal> or <literal>ALTER DATABASE</literal>; + in that mode, only the failed <acronym>SQL</acronym> statement is + rolled back, and the transaction is automatically put back in normal + mode. + </para> + + <para> The transaction access mode determines whether the transaction is read/write or read-only. Read/write is the default. When a transaction is read-only, the following SQL commands are diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index d967400384..c6fd975b7b 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -73,6 +73,8 @@ int DefaultXactIsoLevel = XACT_READ_COMMITTED; int XactIsoLevel; +int XactRollbackScope = XACT_ROLLBACK_SCOPE_XACT; + bool DefaultXactReadOnly = false; bool XactReadOnly; @@ -176,6 +178,7 @@ typedef struct TransactionStateData int savepointLevel; /* savepoint level */ TransState state; /* low-level state */ TBlockState blockState; /* high-level state */ + XactRollbackScopeValues rollbackScope; /* automatic abort behavior */ int nestingLevel; /* transaction nesting depth */ int gucNestLevel; /* GUC context nesting depth */ MemoryContext curTransactionContext; /* my xact-lifetime context */ @@ -202,6 +205,7 @@ typedef TransactionStateData *TransactionState; static TransactionStateData TopTransactionStateData = { .state = TRANS_DEFAULT, .blockState = TBLOCK_DEFAULT, + .rollbackScope = XACT_ROLLBACK_SCOPE_XACT, }; /* @@ -1820,6 +1824,8 @@ StartTransaction(void) */ s->state = TRANS_START; s->transactionId = InvalidTransactionId; /* until assigned */ + /* fixed later in CommitTransactionCommand: */ + s->rollbackScope = XACT_ROLLBACK_SCOPE_XACT; /* * initialize current transaction state fields @@ -2705,6 +2711,7 @@ StartTransactionCommand(void) */ case TBLOCK_DEFAULT: StartTransaction(); + s = CurrentTransactionState; s->blockState = TBLOCK_STARTED; break; @@ -2717,7 +2724,23 @@ StartTransactionCommand(void) */ case TBLOCK_INPROGRESS: case TBLOCK_IMPLICIT_INPROGRESS: + break; + + /* + * Same as above in transaction rollback scope; but in statement + * rollback scope, release the previous subtransaction and create + * a new one. + */ case TBLOCK_SUBINPROGRESS: + if (s->rollbackScope == XACT_ROLLBACK_SCOPE_STMT) + { + CommitSubTransaction(); /* what if there's an intervening subxact? */ + PushTransaction(); + s = CurrentTransactionState; + s->name = MemoryContextStrdup(TopTransactionContext, "pg internal"); + StartSubTransaction(); + s->blockState = TBLOCK_SUBINPROGRESS; + } break; /* @@ -2792,13 +2815,25 @@ CommitTransactionCommand(void) break; /* - * We are completing a "BEGIN TRANSACTION" command, so we change - * to the "transaction block in progress" state and return. (We - * assume the BEGIN did nothing to the database, so we need no - * CommandCounterIncrement.) + * While completing a BEGIN transaction command, we mind the + * prevailing transaction rollback scope. In the normal case of + * rolling back the entire transaction, just change to the + * "transaction block in progress" state and return. (We assume + * the BEGIN did nothing to the database, so we need no + * CommandCounterIncrement.) If the rollback scope is statement, + * however, here we start a subtransaction to serve it. */ case TBLOCK_BEGIN: + s->rollbackScope = XactRollbackScope; s->blockState = TBLOCK_INPROGRESS; + if (s->rollbackScope == XACT_ROLLBACK_SCOPE_STMT) + { + PushTransaction(); + s = CurrentTransactionState; /* changed by push */ + s->name = MemoryContextStrdup(TopTransactionContext, "pg internal"); + StartSubTransaction(); + s->blockState = TBLOCK_SUBINPROGRESS; + } break; /* @@ -2985,6 +3020,7 @@ CommitTransactionCommand(void) savepointLevel = s->savepointLevel; CleanupSubTransaction(); + s = CurrentTransactionState; DefineSavepoint(NULL); s = CurrentTransactionState; /* changed by push */ @@ -3063,6 +3099,7 @@ AbortCurrentTransaction(void) */ case TBLOCK_INPROGRESS: case TBLOCK_PARALLEL_INPROGRESS: + Assert(s->rollbackScope != XACT_ROLLBACK_SCOPE_STMT); AbortTransaction(); s->blockState = TBLOCK_ABORT; /* CleanupTransaction happens when we exit TBLOCK_ABORT_END */ @@ -3120,13 +3157,32 @@ AbortCurrentTransaction(void) break; /* - * We got an error inside a subtransaction. Abort just the - * subtransaction, and go to the persistent SUBABORT state until - * we get ROLLBACK. + * We got an error inside a subtransaction. In transaction- + * rollback-scope mode, abort just the subtransaction, and go to + * the persistent SUBABORT state until we get ROLLBACK; in + * statement rollback-scope mode, abort and cleanup the current + * subtransaction and automatically start a new one. */ case TBLOCK_SUBINPROGRESS: AbortSubTransaction(); - s->blockState = TBLOCK_SUBABORT; + + if (s->rollbackScope == XACT_ROLLBACK_SCOPE_STMT) + { + char *name = s->name; + + s->name = NULL; + CleanupSubTransaction(); + + PushTransaction(); + s = CurrentTransactionState; /* changed by push */ + s->name = name; + + AssertState(s->blockState == TBLOCK_SUBBEGIN); + StartSubTransaction(); + s->blockState = TBLOCK_SUBINPROGRESS; + } + else + s->blockState = TBLOCK_SUBABORT; break; /* @@ -3863,9 +3919,8 @@ DefineSavepoint(const char *name) switch (s->blockState) { - case TBLOCK_INPROGRESS: - case TBLOCK_SUBINPROGRESS: /* Normal subtransaction start */ + case TBLOCK_INPROGRESS: PushTransaction(); s = CurrentTransactionState; /* changed by push */ @@ -3878,6 +3933,50 @@ DefineSavepoint(const char *name) break; /* + * As above, with a subtransaction already in progress. + * + * In transaction-scope rollback, we want the user-defined + * savepoint to appear *before* the automatic subtransaction. + * Close the one we opened above, then open and start the user + * invoked one, then start a new automatic savepoint, which is + * the one that CommitTransactionCommand will act upon. + * + * In normal mode, act just like the INPROGRESS case. + */ + case TBLOCK_SUBINPROGRESS: + if (s->rollbackScope == XACT_ROLLBACK_SCOPE_STMT) + { + CommitSubTransaction(); + PushTransaction(); + s = CurrentTransactionState; /* changed by push */ + if (name) + s->name = MemoryContextStrdup(TopTransactionContext, name); + StartSubTransaction(); + s->blockState = TBLOCK_SUBINPROGRESS; + + /* same as StartTransactionCommand for SUBINPROGRESS */ + PushTransaction(); + s = CurrentTransactionState; + s->name = MemoryContextStrdup(TopTransactionContext, "pg internal"); + StartSubTransaction(); + s->blockState = TBLOCK_SUBINPROGRESS; + + } + else + { + PushTransaction(); + s = CurrentTransactionState; /* changed by push */ + + /* + * Savepoint names, like the TransactionState block itself, live + * in TopTransactionContext. + */ + if (name) + s->name = MemoryContextStrdup(TopTransactionContext, name); + } + break; + + /* * We disallow savepoint commands in implicit transaction blocks. * There would be no great difficulty in allowing them so far as * this module is concerned, but a savepoint seems inconsistent @@ -4547,6 +4646,7 @@ StartSubTransaction(void) TransStateAsString(s->state)); s->state = TRANS_START; + s->rollbackScope = s->parent->rollbackScope; /* * Initialize subsystems for new subtransaction diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c index c7e5a9ca9f..c2af9b9795 100644 --- a/src/backend/commands/variable.c +++ b/src/backend/commands/variable.c @@ -555,6 +555,37 @@ check_XactIsoLevel(int *newval, void **extra, GucSource source) } /* + * SET TRANSACTION ROLLBACK SCOPE + */ +bool +check_XactRollbackScope(int *newval, void **extra, GucSource source) +{ + /* idempotent changes are okay */ + if (*newval == XactRollbackScope) + return true; + + /* + * if in statement mode, allow downgrade to transaction mode (no going + * back from this one!) + */ + if ((*newval == XACT_ROLLBACK_SCOPE_XACT) && + (XactRollbackScope == XACT_ROLLBACK_SCOPE_STMT)) + return true; + + /* + * We want to reject the variable unless it comes from ALTER + * USER/DATABASE or connection establishment. We don't want to + * accept it in postgresql.conf or in manual SET. + */ + if (source < PGC_S_SESSION && source > PGC_S_GLOBAL) + return true; + + GUC_check_errmsg("the transaction rollback scope cannot be changed at this level"); + GUC_check_errhint("You may set it in PGOPTIONS or via ALTER USER/DATABASE SET."); + return false; +} + +/* * SET TRANSACTION [NOT] DEFERRABLE */ diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 2c2208ffb7..1f924101fb 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -353,7 +353,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type <node> RowSecurityOptionalWithCheck RowSecurityOptionalExpr %type <list> RowSecurityDefaultToRole RowSecurityOptionalToRole -%type <str> iso_level opt_encoding +%type <str> iso_level rollback_scope opt_encoding %type <rolespec> grantee %type <list> grantee_list %type <accesspriv> privilege @@ -673,7 +673,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); RESET RESTART RESTRICT RETURNING RETURNS REVOKE RIGHT ROLE ROLLBACK ROLLUP ROUTINE ROUTINES ROW ROWS RULE - SAVEPOINT SCHEMA SCHEMAS SCROLL SEARCH SECOND_P SECURITY SELECT SEQUENCE SEQUENCES + SAVEPOINT SCHEMA SCHEMAS SCOPE SCROLL SEARCH SECOND_P SECURITY SELECT SEQUENCE SEQUENCES SERIALIZABLE SERVER SESSION SESSION_USER SET SETS SETOF SHARE SHOW SIMILAR SIMPLE SKIP SMALLINT SNAPSHOT SOME SQL_P STABLE STANDALONE_P START STATEMENT STATISTICS STDIN STDOUT STORAGE STRICT_P STRIP_P @@ -1578,6 +1578,10 @@ iso_level: READ UNCOMMITTED { $$ = "read uncommitted"; } | SERIALIZABLE { $$ = "serializable"; } ; +rollback_scope: TRANSACTION { $$ = "transaction"; } + | STATEMENT { $$ = "statement"; } + ; + opt_boolean_or_string: TRUE_P { $$ = "true"; } | FALSE_P { $$ = "false"; } @@ -9916,6 +9920,9 @@ transaction_mode_item: ISOLATION LEVEL iso_level { $$ = makeDefElem("transaction_isolation", makeStringConst($3, @3), @1); } + | ROLLBACK SCOPE rollback_scope + { $$ = makeDefElem("transaction_rollback_scope", + makeStringConst($3, @3), @1); } | READ ONLY { $$ = makeDefElem("transaction_read_only", makeIntConst(true, @1), @1); } @@ -15184,6 +15191,7 @@ unreserved_keyword: | SAVEPOINT | SCHEMA | SCHEMAS + | SCOPE | SCROLL | SEARCH | SECOND_P diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 970c94ee80..3077a9adb5 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -427,6 +427,10 @@ standard_ProcessUtility(PlannedStmt *pstmt, SetPGVariable("transaction_isolation", list_make1(item->arg), true); + if (strcmp(item->defname, "transaction_rollback_scope") == 0) + SetPGVariable("transaction_rollback_scope", + list_make1(item->arg), + true); else if (strcmp(item->defname, "transaction_read_only") == 0) SetPGVariable("transaction_read_only", list_make1(item->arg), diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 6fe1939881..de614d5e05 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -295,6 +295,12 @@ static const struct config_enum_entry isolation_level_options[] = { {NULL, 0} }; +static const struct config_enum_entry rollback_scope_options[] = { + {"transaction", XACT_ROLLBACK_SCOPE_XACT, false}, + {"statement", XACT_ROLLBACK_SCOPE_STMT, false}, + {NULL, 0} +}; + static const struct config_enum_entry session_replication_role_options[] = { {"origin", SESSION_REPLICATION_ROLE_ORIGIN, false}, {"replica", SESSION_REPLICATION_ROLE_REPLICA, false}, @@ -4151,6 +4157,17 @@ static struct config_enum ConfigureNamesEnum[] = }, { + {"transaction_rollback_scope", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Sets the scope of rollback when the current transaction aborts."), + NULL, + GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE + }, + &XactRollbackScope, + XACT_ROLLBACK_SCOPE_XACT, rollback_scope_options, + check_XactRollbackScope, NULL, NULL + }, + + { {"IntervalStyle", PGC_USERSET, CLIENT_CONN_LOCALE, gettext_noop("Sets the display format for interval values."), NULL, @@ -7853,6 +7870,9 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel) else if (strcmp(item->defname, "transaction_read_only") == 0) SetPGVariable("transaction_read_only", list_make1(item->arg), stmt->is_local); + else if (strcmp(item->defname, "transaction_rollback_scope") == 0) + SetPGVariable("transaction_rollback_scope", + list_make1(item->arg), stmt->is_local); else if (strcmp(item->defname, "transaction_deferrable") == 0) SetPGVariable("transaction_deferrable", list_make1(item->arg), stmt->is_local); diff --git a/src/include/access/xact.h b/src/include/access/xact.h index 689c57c592..dad8bb172d 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -50,6 +50,17 @@ extern PGDLLIMPORT int XactIsoLevel; #define IsolationUsesXactSnapshot() (XactIsoLevel >= XACT_REPEATABLE_READ) #define IsolationIsSerializable() (XactIsoLevel == XACT_SERIALIZABLE) +/* + * Transaction rollback scope + */ +typedef enum +{ + XACT_ROLLBACK_SCOPE_XACT, + XACT_ROLLBACK_SCOPE_STMT +} XactRollbackScopeValues; + +extern PGDLLIMPORT int XactRollbackScope; + /* Xact read-only state */ extern bool DefaultXactReadOnly; extern bool XactReadOnly; diff --git a/src/include/commands/variable.h b/src/include/commands/variable.h index 7373a3f99f..6239ee1788 100644 --- a/src/include/commands/variable.h +++ b/src/include/commands/variable.h @@ -23,6 +23,7 @@ extern void assign_log_timezone(const char *newval, void *extra); extern const char *show_log_timezone(void); extern bool check_transaction_read_only(bool *newval, void **extra, GucSource source); extern bool check_XactIsoLevel(int *newval, void **extra, GucSource source); +extern bool check_XactRollbackScope(int *newval, void **extra, GucSource source); extern bool check_transaction_deferrable(bool *newval, void **extra, GucSource source); extern bool check_random_seed(double *newval, void **extra, GucSource source); extern void assign_random_seed(double newval, void *extra); diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h index 23db40147b..cd739e1149 100644 --- a/src/include/parser/kwlist.h +++ b/src/include/parser/kwlist.h @@ -353,6 +353,7 @@ PG_KEYWORD("rule", RULE, UNRESERVED_KEYWORD) PG_KEYWORD("savepoint", SAVEPOINT, UNRESERVED_KEYWORD) PG_KEYWORD("schema", SCHEMA, UNRESERVED_KEYWORD) PG_KEYWORD("schemas", SCHEMAS, UNRESERVED_KEYWORD) +PG_KEYWORD("scope", SCOPE, UNRESERVED_KEYWORD) PG_KEYWORD("scroll", SCROLL, UNRESERVED_KEYWORD) PG_KEYWORD("search", SEARCH, UNRESERVED_KEYWORD) PG_KEYWORD("second", SECOND_P, UNRESERVED_KEYWORD) diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out index 69e176c525..1a1a28be67 100644 --- a/src/test/regress/expected/transactions.out +++ b/src/test/regress/expected/transactions.out @@ -659,6 +659,76 @@ ERROR: portal "ctt" cannot be run COMMIT; DROP FUNCTION create_temp_tab(); DROP FUNCTION invert(x float8); +-- tests for statement-scope rollback +select current_user \gset +CREATE USER regress_transactions_user; +ALTER USER regress_transactions_user SET transaction_rollback_scope TO statement; +\c - regress_transactions_user +SHOW transaction_rollback_scope; + transaction_rollback_scope +---------------------------- + statement +(1 row) + +CREATE TABLE xact_rscope (a int); +-- elementary test: three separate insertions in a transaction, an abort in the +-- middle one. We expect the other two to succeed. +BEGIN; +INSERT INTO xact_rscope VALUES (1); +INSERT INTO xact_rscope VALUES (0/0); +ERROR: division by zero +INSERT INTO xact_rscope VALUES (2); +COMMIT; +SELECT * FROM xact_rscope; + a +--- + 1 + 2 +(2 rows) + +-- DO blocks don't have this behavior +DO $$ +BEGIN + INSERT INTO xact_rscope VALUES (3); + INSERT INTO xact_rscope VALUES (4/0); + INSERT INTO xact_rscope VALUES (5); +END; $$; +ERROR: division by zero +CONTEXT: SQL statement "INSERT INTO xact_rscope VALUES (4/0)" +PL/pgSQL function inline_code_block line 4 at SQL statement +SELECT * FROM xact_rscope; + a +--- + 1 + 2 +(2 rows) + +-- implicit transaction blocks don't have this behavior +INSERT INTO xact_rscope VALUES (6)\; INSERT INTO xact_rscope VALUES (7/0)\; INSERT INTO xact_rscope VALUES (8); +ERROR: division by zero +-- test savepoints +BEGIN; +INSERT INTO xact_rscope VALUES (9); +SAVEPOINT xact_svpt; +INSERT INTO xact_rscope VALUES (10); +INSERT INTO xact_rscope VALUES (11/0); +ERROR: division by zero +ROLLBACK TO xact_svpt; +INSERT INTO xact_rscope VALUES (12); +COMMIT; +SELECT * FROM xact_rscope; + a +---- + 1 + 2 + 9 + 12 +(4 rows) + +-- clean up +\c - :current_user +DROP OWNED BY regress_transactions_user; +DROP USER regress_transactions_user; -- 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 diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql index 2e3739fd6c..3ac3450545 100644 --- a/src/test/regress/sql/transactions.sql +++ b/src/test/regress/sql/transactions.sql @@ -418,6 +418,52 @@ COMMIT; DROP FUNCTION create_temp_tab(); DROP FUNCTION invert(x float8); +-- tests for statement-scope rollback +select current_user \gset +CREATE USER regress_transactions_user; +ALTER USER regress_transactions_user SET transaction_rollback_scope TO statement; +\c - regress_transactions_user +SHOW transaction_rollback_scope; + +CREATE TABLE xact_rscope (a int); + +-- elementary test: three separate insertions in a transaction, an abort in the +-- middle one. We expect the other two to succeed. +BEGIN; +INSERT INTO xact_rscope VALUES (1); +INSERT INTO xact_rscope VALUES (0/0); +INSERT INTO xact_rscope VALUES (2); +COMMIT; +SELECT * FROM xact_rscope; + +-- DO blocks don't have this behavior +DO $$ +BEGIN + INSERT INTO xact_rscope VALUES (3); + INSERT INTO xact_rscope VALUES (4/0); + INSERT INTO xact_rscope VALUES (5); +END; $$; +SELECT * FROM xact_rscope; + +-- implicit transaction blocks don't have this behavior +INSERT INTO xact_rscope VALUES (6)\; INSERT INTO xact_rscope VALUES (7/0)\; INSERT INTO xact_rscope VALUES (8); + +-- test savepoints +BEGIN; +INSERT INTO xact_rscope VALUES (9); +SAVEPOINT xact_svpt; +INSERT INTO xact_rscope VALUES (10); +INSERT INTO xact_rscope VALUES (11/0); +ROLLBACK TO xact_svpt; +INSERT INTO xact_rscope VALUES (12); +COMMIT; +SELECT * FROM xact_rscope; + +-- clean up +\c - :current_user +DROP OWNED BY regress_transactions_user; +DROP USER regress_transactions_user; + -- Test assorted behaviors around the implicit transaction block created -- when multiple SQL commands are sent in a single Query message. These -- 2.11.0
>From 671de0c55ba20133b858933e02346327c65791a4 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Fri, 7 Dec 2018 15:57:28 -0300 Subject: [PATCH 2/2] Remove restrictions on setting rollback scope --- doc/src/sgml/ref/set_transaction.sgml | 11 +- src/backend/access/transam/xact.c | 39 ++++++- src/backend/commands/variable.c | 31 ------ src/backend/utils/misc/guc.c | 2 +- src/include/commands/variable.h | 4 +- src/test/regress/expected/transactions.out | 170 +++++++++++++++++++++++++++++ src/test/regress/sql/transactions.sql | 95 ++++++++++++++++ 7 files changed, 311 insertions(+), 41 deletions(-) diff --git a/doc/src/sgml/ref/set_transaction.sgml b/doc/src/sgml/ref/set_transaction.sgml index e6aa4f46f6..ef94e57ef6 100644 --- a/doc/src/sgml/ref/set_transaction.sgml +++ b/doc/src/sgml/ref/set_transaction.sgml @@ -134,13 +134,10 @@ SET SESSION CHARACTERISTICS AS TRANSACTION <replaceable class="parameter">transa The rollback scope of a transaction determines the range of operations which roll back when an SQL statement fails. The default value is <literal>TRANSACTION</literal>, which causes the entire transaction or - current subtransaction to be rolled back. This is the only mode that can - be selected with <command>SET TRANSACTION</command>. The other possible - mode, <literal>STATEMENT</literal>, can only be specified during connection - establishment, <literal>ALTER USER</literal> or <literal>ALTER DATABASE</literal>; - in that mode, only the failed <acronym>SQL</acronym> statement is - rolled back, and the transaction is automatically put back in normal - mode. + current subtransaction to be rolled back. The other possible mode is + <literal>STATEMENT</literal>, in which only the failed + <acronym>SQL</acronym> statement is rolled back, and the transaction + is automatically put back in normal mode. </para> <para> diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index c6fd975b7b..b30683eb9f 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -36,6 +36,7 @@ #include "commands/async.h" #include "commands/tablecmds.h" #include "commands/trigger.h" +#include "commands/variable.h" #include "executor/spi.h" #include "libpq/be-fsstubs.h" #include "libpq/pqsignal.h" @@ -2722,10 +2723,26 @@ StartTransactionCommand(void) * that any needed CommandCounterIncrement was done by the * previous CommitTransactionCommand.) */ - case TBLOCK_INPROGRESS: case TBLOCK_IMPLICIT_INPROGRESS: break; + /* Normally there's nothing to do for the in-progress case. But in + * statement-scoped rollback mode, we start a new subtransaction + * here to which we can roll back. This case is pretty rare: it + * only occurs when the GUC is changed partway through a + * transaction. + */ + case TBLOCK_INPROGRESS: + if (s->rollbackScope == XACT_ROLLBACK_SCOPE_STMT) + { + PushTransaction(); + s = CurrentTransactionState; + s->name = MemoryContextStrdup(TopTransactionContext, "pg internal"); + StartSubTransaction(); + s->blockState = TBLOCK_SUBINPROGRESS; + } + break; + /* * Same as above in transaction rollback scope; but in statement * rollback scope, release the previous subtransaction and create @@ -5243,6 +5260,26 @@ ShowTransactionStateRec(const char *str, TransactionState s) } /* + * GUC assign hook for SET TRANSACTION ROLLBACK SCOPE + */ +void +assign_XactRollbackScope(int newval, void *extra) +{ + TransactionState s = CurrentTransactionState; + + /* + * Alter the GUC setting for future transactions ... + */ + XactRollbackScope = newval; + + /* ... as well as update the current transaction's status */ + if ((s->state == TRANS_INPROGRESS) && (newval != s->rollbackScope)) + s->rollbackScope = newval; + + /* Should we prepare transaction state here? */ +} + +/* * BlockStateAsString * Debug support */ diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c index c2af9b9795..c7e5a9ca9f 100644 --- a/src/backend/commands/variable.c +++ b/src/backend/commands/variable.c @@ -555,37 +555,6 @@ check_XactIsoLevel(int *newval, void **extra, GucSource source) } /* - * SET TRANSACTION ROLLBACK SCOPE - */ -bool -check_XactRollbackScope(int *newval, void **extra, GucSource source) -{ - /* idempotent changes are okay */ - if (*newval == XactRollbackScope) - return true; - - /* - * if in statement mode, allow downgrade to transaction mode (no going - * back from this one!) - */ - if ((*newval == XACT_ROLLBACK_SCOPE_XACT) && - (XactRollbackScope == XACT_ROLLBACK_SCOPE_STMT)) - return true; - - /* - * We want to reject the variable unless it comes from ALTER - * USER/DATABASE or connection establishment. We don't want to - * accept it in postgresql.conf or in manual SET. - */ - if (source < PGC_S_SESSION && source > PGC_S_GLOBAL) - return true; - - GUC_check_errmsg("the transaction rollback scope cannot be changed at this level"); - GUC_check_errhint("You may set it in PGOPTIONS or via ALTER USER/DATABASE SET."); - return false; -} - -/* * SET TRANSACTION [NOT] DEFERRABLE */ diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index de614d5e05..a8da9decf9 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -4164,7 +4164,7 @@ static struct config_enum ConfigureNamesEnum[] = }, &XactRollbackScope, XACT_ROLLBACK_SCOPE_XACT, rollback_scope_options, - check_XactRollbackScope, NULL, NULL + NULL, assign_XactRollbackScope, NULL }, { diff --git a/src/include/commands/variable.h b/src/include/commands/variable.h index 6239ee1788..9cd8131e5b 100644 --- a/src/include/commands/variable.h +++ b/src/include/commands/variable.h @@ -23,7 +23,6 @@ extern void assign_log_timezone(const char *newval, void *extra); extern const char *show_log_timezone(void); extern bool check_transaction_read_only(bool *newval, void **extra, GucSource source); extern bool check_XactIsoLevel(int *newval, void **extra, GucSource source); -extern bool check_XactRollbackScope(int *newval, void **extra, GucSource source); extern bool check_transaction_deferrable(bool *newval, void **extra, GucSource source); extern bool check_random_seed(double *newval, void **extra, GucSource source); extern void assign_random_seed(double newval, void *extra); @@ -36,4 +35,7 @@ extern bool check_role(char **newval, void **extra, GucSource source); extern void assign_role(const char *newval, void *extra); extern const char *show_role(void); +/* in xact.c */ +extern void assign_XactRollbackScope(int newval, void *extra); + #endif /* VARIABLE_H */ diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out index 1a1a28be67..a259cd6121 100644 --- a/src/test/regress/expected/transactions.out +++ b/src/test/regress/expected/transactions.out @@ -725,10 +725,180 @@ SELECT * FROM xact_rscope; 12 (4 rows) +-- test changing rollback scope to 'transaction' partway through +TRUNCATE TABLE xact_rscope; +BEGIN; +SHOW transaction_rollback_scope; + transaction_rollback_scope +---------------------------- + statement +(1 row) + +INSERT INTO xact_rscope VALUES (1); +INSERT INTO xact_rscope VALUES (2/0); +ERROR: division by zero +SET LOCAL transaction_rollback_scope TO 'transaction'; +INSERT INTO xact_rscope VALUES (3); +COMMIT; +SELECT * FROM xact_rscope; + a +--- + 1 + 3 +(2 rows) + +TRUNCATE TABLE xact_rscope; +BEGIN; +INSERT INTO xact_rscope VALUES (1); +INSERT INTO xact_rscope VALUES (2/0); +ERROR: division by zero +SET LOCAL transaction_rollback_scope TO 'transaction'; +INSERT INTO xact_rscope VALUES (3); +INSERT INTO xact_rscope VALUES (4/0); +ERROR: division by zero +COMMIT; +SELECT * FROM xact_rscope; + a +--- +(0 rows) + +TRUNCATE TABLE xact_rscope; +BEGIN; +INSERT INTO xact_rscope VALUES (1); +SAVEPOINT foo; +INSERT INTO xact_rscope VALUES (2/0); +ERROR: division by zero +SET LOCAL transaction_rollback_scope TO 'transaction'; +INSERT INTO xact_rscope VALUES (4/0); +ERROR: division by zero +ROLLBACK TO foo; +INSERT INTO xact_rscope VALUES (5); +COMMIT; +SELECT * FROM xact_rscope; + a +--- + 1 + 5 +(2 rows) + +TRUNCATE TABLE xact_rscope; +BEGIN; +INSERT INTO xact_rscope VALUES (1); +SAVEPOINT foo; +INSERT INTO xact_rscope VALUES (2/0); +ERROR: division by zero +SET LOCAL transaction_rollback_scope TO 'transaction'; +INSERT INTO xact_rscope VALUES (4); +RELEASE SAVEPOINT foo; +INSERT INTO xact_rscope VALUES (5); +COMMIT; +SELECT * FROM xact_rscope; + a +--- + 1 + 4 + 5 +(3 rows) + -- clean up \c - :current_user DROP OWNED BY regress_transactions_user; DROP USER regress_transactions_user; +-- We allow the GUC to be changed mid-session too +CREATE TABLE xact_rscope (a int); +SHOW transaction_rollback_scope; + transaction_rollback_scope +---------------------------- + transaction +(1 row) + +BEGIN TRANSACTION ROLLBACK SCOPE STATEMENT; +SHOW transaction_rollback_scope; + transaction_rollback_scope +---------------------------- + statement +(1 row) + +INSERT INTO xact_rscope VALUES (1); +INSERT INTO xact_rscope VALUES (2/0); +ERROR: division by zero +INSERT INTO xact_rscope VALUES (3), (4/0); +ERROR: division by zero +INSERT INTO xact_rscope VALUES (5); +COMMIT; +SELECT * FROM xact_rscope; + a +--- + 1 + 5 +(2 rows) + +TRUNCATE TABLE xact_rscope; +SET transaction_rollback_scope TO 'statement'; +BEGIN; +INSERT INTO xact_rscope VALUES (5); +INSERT INTO xact_rscope VALUES (6/0); +ERROR: division by zero +COMMIT; +SELECT * FROM xact_rscope; + a +--- + 5 +(1 row) + +RESET transaction_rollback_scope; +TRUNCATE TABLE xact_rscope; +BEGIN; +SHOW transaction_rollback_scope; + transaction_rollback_scope +---------------------------- + transaction +(1 row) + +INSERT INTO xact_rscope VALUES (1); +SET LOCAL transaction_rollback_scope TO 'statement'; +INSERT INTO xact_rscope VALUES (2/0); +ERROR: division by zero +INSERT INTO xact_rscope VALUES (3); +COMMIT; +SELECT * FROM xact_rscope; + a +--- + 1 + 3 +(2 rows) + +SHOW transaction_rollback_scope; + transaction_rollback_scope +---------------------------- + transaction +(1 row) + +TRUNCATE TABLE xact_rscope; +BEGIN ROLLBACK SCOPE STATEMENT; +INSERT INTO xact_rscope VALUES (1); +SAVEPOINT foo; +INSERT INTO xact_rscope VALUES (2/0); +ERROR: division by zero +INSERT INTO xact_rscope VALUES (3); +RELEASE SAVEPOINT foo; +INSERT INTO xact_rscope VALUES (4); +SAVEPOINT bar; +INSERT INTO xact_rscope VALUES (5); +INSERT INTO xact_rscope VALUES (6/0); +ERROR: division by zero +ROLLBACK TO bar; +INSERT INTO xact_rscope VALUES (7); +COMMIT; +SELECT * FROM xact_rscope; + a +--- + 1 + 3 + 4 + 7 +(4 rows) + -- 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 diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql index 3ac3450545..4fa8246de7 100644 --- a/src/test/regress/sql/transactions.sql +++ b/src/test/regress/sql/transactions.sql @@ -459,11 +459,106 @@ INSERT INTO xact_rscope VALUES (12); COMMIT; SELECT * FROM xact_rscope; +-- test changing rollback scope to 'transaction' partway through +TRUNCATE TABLE xact_rscope; +BEGIN; +SHOW transaction_rollback_scope; +INSERT INTO xact_rscope VALUES (1); +INSERT INTO xact_rscope VALUES (2/0); +SET LOCAL transaction_rollback_scope TO 'transaction'; +INSERT INTO xact_rscope VALUES (3); +COMMIT; +SELECT * FROM xact_rscope; + +TRUNCATE TABLE xact_rscope; +BEGIN; +INSERT INTO xact_rscope VALUES (1); +INSERT INTO xact_rscope VALUES (2/0); +SET LOCAL transaction_rollback_scope TO 'transaction'; +INSERT INTO xact_rscope VALUES (3); +INSERT INTO xact_rscope VALUES (4/0); +COMMIT; +SELECT * FROM xact_rscope; + +TRUNCATE TABLE xact_rscope; +BEGIN; +INSERT INTO xact_rscope VALUES (1); +SAVEPOINT foo; +INSERT INTO xact_rscope VALUES (2/0); +SET LOCAL transaction_rollback_scope TO 'transaction'; +INSERT INTO xact_rscope VALUES (4/0); +ROLLBACK TO foo; +INSERT INTO xact_rscope VALUES (5); +COMMIT; +SELECT * FROM xact_rscope; + + +TRUNCATE TABLE xact_rscope; +BEGIN; +INSERT INTO xact_rscope VALUES (1); +SAVEPOINT foo; +INSERT INTO xact_rscope VALUES (2/0); +SET LOCAL transaction_rollback_scope TO 'transaction'; +INSERT INTO xact_rscope VALUES (4); +RELEASE SAVEPOINT foo; +INSERT INTO xact_rscope VALUES (5); +COMMIT; +SELECT * FROM xact_rscope; + -- clean up \c - :current_user DROP OWNED BY regress_transactions_user; DROP USER regress_transactions_user; +-- We allow the GUC to be changed mid-session too +CREATE TABLE xact_rscope (a int); +SHOW transaction_rollback_scope; +BEGIN TRANSACTION ROLLBACK SCOPE STATEMENT; +SHOW transaction_rollback_scope; +INSERT INTO xact_rscope VALUES (1); +INSERT INTO xact_rscope VALUES (2/0); +INSERT INTO xact_rscope VALUES (3), (4/0); +INSERT INTO xact_rscope VALUES (5); +COMMIT; +SELECT * FROM xact_rscope; + +TRUNCATE TABLE xact_rscope; +SET transaction_rollback_scope TO 'statement'; +BEGIN; +INSERT INTO xact_rscope VALUES (5); +INSERT INTO xact_rscope VALUES (6/0); +COMMIT; +SELECT * FROM xact_rscope; +RESET transaction_rollback_scope; + +TRUNCATE TABLE xact_rscope; +BEGIN; +SHOW transaction_rollback_scope; +INSERT INTO xact_rscope VALUES (1); +SET LOCAL transaction_rollback_scope TO 'statement'; +INSERT INTO xact_rscope VALUES (2/0); +INSERT INTO xact_rscope VALUES (3); +COMMIT; +SELECT * FROM xact_rscope; +SHOW transaction_rollback_scope; + +TRUNCATE TABLE xact_rscope; +BEGIN ROLLBACK SCOPE STATEMENT; +INSERT INTO xact_rscope VALUES (1); +SAVEPOINT foo; +INSERT INTO xact_rscope VALUES (2/0); +INSERT INTO xact_rscope VALUES (3); +RELEASE SAVEPOINT foo; +INSERT INTO xact_rscope VALUES (4); +SAVEPOINT bar; +INSERT INTO xact_rscope VALUES (5); +INSERT INTO xact_rscope VALUES (6/0); +ROLLBACK TO bar; +INSERT INTO xact_rscope VALUES (7); +COMMIT; +SELECT * FROM xact_rscope; + + -- Test assorted behaviors around the implicit transaction block created -- when multiple SQL commands are sent in a single Query message. These -- 2.11.0