On 26/12/2018 09:20, Fabien COELHO wrote: > I try to avoid global variables when possible as a principle, because I > paid to learn that they are bad:-) Maybe I'd do a local struct, declare an > instance within the function, and write two functions to dump/restore the > transaction status variables into the referenced struct. Maybe this is not > worth the effort.
Those are reasonable alternatives, I think, but it's a bit overkill in this case. >>> About the tests: I'd suggest to use more options on the different tests, >>> eg SERIALIZABLE, READ ONLY… Also ISTM that tests should show >>> transaction_read_only value as well. >> >> OK, updated a bit. (Using READ ONLY doesn't work because then you can't >> write anything inside the transaction.) > > Sure. Within a read-only tx, it could check that transaction_read_only is > on, and still on when chained, though. I think the tests prove the point that the values are set and unset and reset in various scenarios. We don't need to test every single combination, that's not the job of this patch. > The documentation should probably tell in the compatibility sections that > AND CHAIN is standard conforming. Good point. Updated that. > In the automatic rollback tests, maybe I'd insert 3 just once, and use > another value for the chained transaction. done > Tests may eventually vary BEGIN/START, COMMIT/END, ROLLBACK/ABORT. Those work on the parser level, so don't really affect this patch. It would make the tests more confusing to read, I think. > As you added a SAVEPOINT, maybe I'd try rollbacking to it. That would error out and invalidate the subsequent tests, so it's not easily possible. We could write a totally separate test for it, but I'm not sure what that would be proving. Updated patches attached. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From ec4e153976f21d48f66bde1403d569ce63f8638e Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Wed, 2 Jan 2019 15:09:04 +0100 Subject: [PATCH v4 1/2] Transaction chaining Add command variants COMMIT AND CHAIN and ROLLBACK AND CHAIN, which start new transactions with the same transaction characteristics as the just finished one, per SQL standard. --- doc/src/sgml/ref/abort.sgml | 14 +- doc/src/sgml/ref/commit.sgml | 19 +- doc/src/sgml/ref/end.sgml | 14 +- doc/src/sgml/ref/rollback.sgml | 19 +- src/backend/access/transam/xact.c | 73 +++++++- src/backend/catalog/sql_features.txt | 2 +- src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/equalfuncs.c | 1 + src/backend/parser/gram.y | 19 +- src/backend/tcop/utility.c | 4 +- src/bin/psql/tab-complete.c | 8 +- src/include/access/xact.h | 4 +- src/include/nodes/parsenodes.h | 1 + src/test/regress/expected/transactions.out | 191 +++++++++++++++++++++ src/test/regress/sql/transactions.sql | 63 +++++++ 15 files changed, 408 insertions(+), 25 deletions(-) diff --git a/doc/src/sgml/ref/abort.sgml b/doc/src/sgml/ref/abort.sgml index 21799d2a83..0372913365 100644 --- a/doc/src/sgml/ref/abort.sgml +++ b/doc/src/sgml/ref/abort.sgml @@ -21,7 +21,7 @@ <refsynopsisdiv> <synopsis> -ABORT [ WORK | TRANSACTION ] +ABORT [ WORK | TRANSACTION ] [ AND [ NO ] CHAIN ] </synopsis> </refsynopsisdiv> @@ -51,6 +51,18 @@ <title>Parameters</title> </para> </listitem> </varlistentry> + + <varlistentry> + <term><literal>AND CHAIN</literal></term> + <listitem> + <para> + If <literal>AND CHAIN</literal> is specified, a new transaction is + immediately started with the same transaction characteristics (see <xref + linkend="sql-set-transaction"/>) as the just finished one. Otherwise, + no new transaction is started. + </para> + </listitem> + </varlistentry> </variablelist> </refsect1> diff --git a/doc/src/sgml/ref/commit.sgml b/doc/src/sgml/ref/commit.sgml index b2e8d5d180..96a018e6aa 100644 --- a/doc/src/sgml/ref/commit.sgml +++ b/doc/src/sgml/ref/commit.sgml @@ -21,7 +21,7 @@ <refsynopsisdiv> <synopsis> -COMMIT [ WORK | TRANSACTION ] +COMMIT [ WORK | TRANSACTION ] [ AND [ NO ] CHAIN ] </synopsis> </refsynopsisdiv> @@ -48,6 +48,18 @@ <title>Parameters</title> </para> </listitem> </varlistentry> + + <varlistentry> + <term><literal>AND CHAIN</literal></term> + <listitem> + <para> + If <literal>AND CHAIN</literal> is specified, a new transaction is + immediately started with the same transaction characteristics (see <xref + linkend="sql-set-transaction"/>) as the just finished one. Otherwise, + no new transaction is started. + </para> + </listitem> + </varlistentry> </variablelist> </refsect1> @@ -79,9 +91,8 @@ <title>Examples</title> <title>Compatibility</title> <para> - The SQL standard only specifies the two forms - <literal>COMMIT</literal> and <literal>COMMIT - WORK</literal>. Otherwise, this command is fully conforming. + The command <command>COMMIT</command> conforms to the SQL standard. The + form <literal>COMMIT TRANSACTION</literal> is a PostgreSQL extension. </para> </refsect1> diff --git a/doc/src/sgml/ref/end.sgml b/doc/src/sgml/ref/end.sgml index 7523315f34..8b8f4f0dbb 100644 --- a/doc/src/sgml/ref/end.sgml +++ b/doc/src/sgml/ref/end.sgml @@ -21,7 +21,7 @@ <refsynopsisdiv> <synopsis> -END [ WORK | TRANSACTION ] +END [ WORK | TRANSACTION ] [ AND [ NO ] CHAIN ] </synopsis> </refsynopsisdiv> @@ -50,6 +50,18 @@ <title>Parameters</title> </para> </listitem> </varlistentry> + + <varlistentry> + <term><literal>AND CHAIN</literal></term> + <listitem> + <para> + If <literal>AND CHAIN</literal> is specified, a new transaction is + immediately started with the same transaction characteristics (see <xref + linkend="sql-set-transaction"/>) as the just finished one. Otherwise, + no new transaction is started. + </para> + </listitem> + </varlistentry> </variablelist> </refsect1> diff --git a/doc/src/sgml/ref/rollback.sgml b/doc/src/sgml/ref/rollback.sgml index 3cafb848a9..54fffefe18 100644 --- a/doc/src/sgml/ref/rollback.sgml +++ b/doc/src/sgml/ref/rollback.sgml @@ -21,7 +21,7 @@ <refsynopsisdiv> <synopsis> -ROLLBACK [ WORK | TRANSACTION ] +ROLLBACK [ WORK | TRANSACTION ] [ AND [ NO ] CHAIN ] </synopsis> </refsynopsisdiv> @@ -47,6 +47,18 @@ <title>Parameters</title> </para> </listitem> </varlistentry> + + <varlistentry> + <term><literal>AND CHAIN</literal></term> + <listitem> + <para> + If <literal>AND CHAIN</literal> is specified, a new transaction is + immediately started with the same transaction characteristics (see <xref + linkend="sql-set-transaction"/>) as the just finished one. Otherwise, + no new transaction is started. + </para> + </listitem> + </varlistentry> </variablelist> </refsect1> @@ -78,9 +90,8 @@ <title>Examples</title> <title>Compatibility</title> <para> - The SQL standard only specifies the two forms - <literal>ROLLBACK</literal> and <literal>ROLLBACK - WORK</literal>. Otherwise, this command is fully conforming. + The command <command>ROLLBACK</command> conforms to the SQL standard. The + form <literal>ROLLBACK TRANSACTION</literal> is a PostgreSQL extension. </para> </refsect1> diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index d967400384..6af149d788 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -189,6 +189,7 @@ typedef struct TransactionStateData bool startedInRecovery; /* did we start in recovery? */ bool didLogXid; /* has xid been included in WAL record? */ int parallelModeLevel; /* Enter/ExitParallelMode counter */ + bool chain; /* start a new block after this one */ struct TransactionStateData *parent; /* back link to parent */ } TransactionStateData; @@ -2760,6 +2761,36 @@ StartTransactionCommand(void) MemoryContextSwitchTo(CurTransactionContext); } + +/* + * Simple system for saving and restoring transaction characteristics + * (isolation level, read only, deferrable). We need this for transaction + * chaining, so that we can set the characteristics of the new transaction to + * be the same as the previous one. (We need something like this because the + * GUC system resets the characteristics at transaction end, so for example + * just skipping the reset in StartTransaction() won't work.) + */ +static int save_XactIsoLevel; +static bool save_XactReadOnly; +static bool save_XactDeferrable; + +static void +SaveTransactionCharacteristics(void) +{ + save_XactIsoLevel = XactIsoLevel; + save_XactReadOnly = XactReadOnly; + save_XactDeferrable = XactDeferrable; +} + +static void +RestoreTransactionCharacteristics(void) +{ + XactIsoLevel = save_XactIsoLevel; + XactReadOnly = save_XactReadOnly; + XactDeferrable = save_XactDeferrable; +} + + /* * CommitTransactionCommand */ @@ -2768,6 +2799,9 @@ CommitTransactionCommand(void) { TransactionState s = CurrentTransactionState; + if (s->chain) + SaveTransactionCharacteristics(); + switch (s->blockState) { /* @@ -2819,6 +2853,13 @@ CommitTransactionCommand(void) case TBLOCK_END: CommitTransaction(); s->blockState = TBLOCK_DEFAULT; + if (s->chain) + { + StartTransaction(); + s->blockState = TBLOCK_INPROGRESS; + s->chain = false; + RestoreTransactionCharacteristics(); + } break; /* @@ -2838,6 +2879,13 @@ CommitTransactionCommand(void) case TBLOCK_ABORT_END: CleanupTransaction(); s->blockState = TBLOCK_DEFAULT; + if (s->chain) + { + StartTransaction(); + s->blockState = TBLOCK_INPROGRESS; + s->chain = false; + RestoreTransactionCharacteristics(); + } break; /* @@ -2849,6 +2897,13 @@ CommitTransactionCommand(void) AbortTransaction(); CleanupTransaction(); s->blockState = TBLOCK_DEFAULT; + if (s->chain) + { + StartTransaction(); + s->blockState = TBLOCK_INPROGRESS; + s->chain = false; + RestoreTransactionCharacteristics(); + } break; /* @@ -3506,7 +3561,7 @@ PrepareTransactionBlock(const char *gid) bool result; /* Set up to commit the current transaction */ - result = EndTransactionBlock(); + result = EndTransactionBlock(false); /* If successful, change outer tblock state to PREPARE */ if (result) @@ -3552,7 +3607,7 @@ PrepareTransactionBlock(const char *gid) * resource owner, etc while executing inside a Portal. */ bool -EndTransactionBlock(void) +EndTransactionBlock(bool chain) { TransactionState s = CurrentTransactionState; bool result = false; @@ -3678,6 +3733,13 @@ EndTransactionBlock(void) break; } + Assert(s->blockState == TBLOCK_STARTED || + s->blockState == TBLOCK_END || + s->blockState == TBLOCK_ABORT_END || + s->blockState == TBLOCK_ABORT_PENDING); + + s->chain = chain; + return result; } @@ -3688,7 +3750,7 @@ EndTransactionBlock(void) * As above, we don't actually do anything here except change blockState. */ void -UserAbortTransactionBlock(void) +UserAbortTransactionBlock(bool chain) { TransactionState s = CurrentTransactionState; @@ -3786,6 +3848,11 @@ UserAbortTransactionBlock(void) BlockStateAsString(s->blockState)); break; } + + Assert(s->blockState == TBLOCK_ABORT_END || + s->blockState == TBLOCK_ABORT_PENDING); + + s->chain = chain; } /* diff --git a/src/backend/catalog/sql_features.txt b/src/backend/catalog/sql_features.txt index aeb262a5b0..49df7e08c0 100644 --- a/src/backend/catalog/sql_features.txt +++ b/src/backend/catalog/sql_features.txt @@ -443,7 +443,7 @@ T213 INSTEAD OF triggers YES T231 Sensitive cursors YES T241 START TRANSACTION statement YES T251 SET TRANSACTION statement: LOCAL option NO -T261 Chained transactions NO +T261 Chained transactions YES T271 Savepoints YES T272 Enhanced savepoint management NO T281 SELECT privilege with column granularity YES diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 144cd7a047..35d28c6d7b 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3653,6 +3653,7 @@ _copyTransactionStmt(const TransactionStmt *from) COPY_NODE_FIELD(options); COPY_STRING_FIELD(savepoint_name); COPY_STRING_FIELD(gid); + COPY_SCALAR_FIELD(chain); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index c4579fbcc6..44e5dcc8d7 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1506,6 +1506,7 @@ _equalTransactionStmt(const TransactionStmt *a, const TransactionStmt *b) COMPARE_NODE_FIELD(options); COMPARE_STRING_FIELD(savepoint_name); COMPARE_STRING_FIELD(gid); + COMPARE_SCALAR_FIELD(chain); return true; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 5bcaf42205..f8bc2855b5 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -310,6 +310,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type <boolean> opt_or_replace opt_grant_grant_option opt_grant_admin_option opt_nowait opt_if_exists opt_with_data + opt_transaction_chain %type <ival> opt_nowait_or_skip %type <list> OptRoleList AlterOptRoleList @@ -9805,11 +9806,12 @@ UnlistenStmt: *****************************************************************************/ TransactionStmt: - ABORT_P opt_transaction + ABORT_P opt_transaction opt_transaction_chain { TransactionStmt *n = makeNode(TransactionStmt); n->kind = TRANS_STMT_ROLLBACK; n->options = NIL; + n->chain = $3; $$ = (Node *)n; } | BEGIN_P opt_transaction transaction_mode_list_or_empty @@ -9826,25 +9828,28 @@ TransactionStmt: n->options = $3; $$ = (Node *)n; } - | COMMIT opt_transaction + | COMMIT opt_transaction opt_transaction_chain { TransactionStmt *n = makeNode(TransactionStmt); n->kind = TRANS_STMT_COMMIT; n->options = NIL; + n->chain = $3; $$ = (Node *)n; } - | END_P opt_transaction + | END_P opt_transaction opt_transaction_chain { TransactionStmt *n = makeNode(TransactionStmt); n->kind = TRANS_STMT_COMMIT; n->options = NIL; + n->chain = $3; $$ = (Node *)n; } - | ROLLBACK opt_transaction + | ROLLBACK opt_transaction opt_transaction_chain { TransactionStmt *n = makeNode(TransactionStmt); n->kind = TRANS_STMT_ROLLBACK; n->options = NIL; + n->chain = $3; $$ = (Node *)n; } | SAVEPOINT ColId @@ -9944,6 +9949,12 @@ transaction_mode_list_or_empty: { $$ = NIL; } ; +opt_transaction_chain: + AND CHAIN { $$ = true; } + | AND NO CHAIN { $$ = false; } + | /* EMPTY */ { $$ = false; } + ; + /***************************************************************************** * diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 970c94ee80..cd5c536563 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -440,7 +440,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case TRANS_STMT_COMMIT: - if (!EndTransactionBlock()) + if (!EndTransactionBlock(stmt->chain)) { /* report unsuccessful commit in completionTag */ if (completionTag) @@ -471,7 +471,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case TRANS_STMT_ROLLBACK: - UserAbortTransactionBlock(); + UserAbortTransactionBlock(stmt->chain); break; case TRANS_STMT_SAVEPOINT: diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 91df96e796..1736bdcc2d 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2077,16 +2077,18 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("WORK", "TRANSACTION", "ISOLATION LEVEL", "READ", "DEFERRABLE", "NOT DEFERRABLE"); /* END, ABORT */ else if (Matches("END|ABORT")) - COMPLETE_WITH("WORK", "TRANSACTION"); + COMPLETE_WITH("AND", "WORK", "TRANSACTION"); /* COMMIT */ else if (Matches("COMMIT")) - COMPLETE_WITH("WORK", "TRANSACTION", "PREPARED"); + COMPLETE_WITH("AND", "WORK", "TRANSACTION", "PREPARED"); /* RELEASE SAVEPOINT */ else if (Matches("RELEASE")) COMPLETE_WITH("SAVEPOINT"); /* ROLLBACK */ else if (Matches("ROLLBACK")) - COMPLETE_WITH("WORK", "TRANSACTION", "TO SAVEPOINT", "PREPARED"); + COMPLETE_WITH("AND", "WORK", "TRANSACTION", "TO SAVEPOINT", "PREPARED"); + else if (Matches("ABORT|END|COMMIT|ROLLBACK", "AND")) + COMPLETE_WITH("CHAIN"); /* CALL */ else if (Matches("CALL")) COMPLETE_WITH_VERSIONED_SCHEMA_QUERY(Query_for_list_of_procedures, NULL); diff --git a/src/include/access/xact.h b/src/include/access/xact.h index 689c57c592..c757146e4d 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -372,9 +372,9 @@ extern void StartTransactionCommand(void); extern void CommitTransactionCommand(void); extern void AbortCurrentTransaction(void); extern void BeginTransactionBlock(void); -extern bool EndTransactionBlock(void); +extern bool EndTransactionBlock(bool chain); extern bool PrepareTransactionBlock(const char *gid); -extern void UserAbortTransactionBlock(void); +extern void UserAbortTransactionBlock(bool chain); extern void BeginImplicitTransactionBlock(void); extern void EndImplicitTransactionBlock(void); extern void ReleaseSavepoint(const char *name); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index d3dd3d0339..4010995514 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2985,6 +2985,7 @@ typedef struct TransactionStmt List *options; /* for BEGIN/START commands */ char *savepoint_name; /* for savepoint commands */ char *gid; /* for two-phase-commit related commands */ + bool chain; /* AND CHAIN option */ } TransactionStmt; /* ---------------------- diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out index 69e176c525..1b316cc9b8 100644 --- a/src/test/regress/expected/transactions.out +++ b/src/test/regress/expected/transactions.out @@ -659,6 +659,197 @@ ERROR: portal "ctt" cannot be run COMMIT; DROP FUNCTION create_temp_tab(); DROP FUNCTION invert(x float8); +-- Tests for AND CHAIN +CREATE TABLE abc (a int); +-- set nondefault value so we have something to override below +SET default_transaction_read_only = on; +START TRANSACTION ISOLATION LEVEL REPEATABLE READ, READ WRITE, DEFERRABLE; +SHOW transaction_isolation; + transaction_isolation +----------------------- + repeatable read +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + on +(1 row) + +INSERT INTO abc VALUES (1); +INSERT INTO abc VALUES (2); +COMMIT AND CHAIN; -- TBLOCK_END +SHOW transaction_isolation; + transaction_isolation +----------------------- + repeatable read +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + on +(1 row) + +INSERT INTO abc VALUES ('error'); +ERROR: invalid input syntax for type integer: "error" +LINE 1: INSERT INTO abc VALUES ('error'); + ^ +INSERT INTO abc VALUES (3); -- check it's really aborted +ERROR: current transaction is aborted, commands ignored until end of transaction block +COMMIT AND CHAIN; -- TBLOCK_ABORT_END +SHOW transaction_isolation; + transaction_isolation +----------------------- + repeatable read +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + on +(1 row) + +INSERT INTO abc VALUES (4); +COMMIT; +START TRANSACTION ISOLATION LEVEL REPEATABLE READ, READ WRITE, DEFERRABLE; +SHOW transaction_isolation; + transaction_isolation +----------------------- + repeatable read +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + on +(1 row) + +SAVEPOINT x; +INSERT INTO abc VALUES ('error'); +ERROR: invalid input syntax for type integer: "error" +LINE 1: INSERT INTO abc VALUES ('error'); + ^ +COMMIT AND CHAIN; -- TBLOCK_ABORT_PENDING +SHOW transaction_isolation; + transaction_isolation +----------------------- + repeatable read +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + on +(1 row) + +INSERT INTO abc VALUES (5); +COMMIT; +-- different mix of options just for fun +START TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ WRITE, NOT DEFERRABLE; +SHOW transaction_isolation; + transaction_isolation +----------------------- + serializable +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + off +(1 row) + +INSERT INTO abc VALUES (6); +ROLLBACK AND CHAIN; -- TBLOCK_ABORT_PENDING +SHOW transaction_isolation; + transaction_isolation +----------------------- + serializable +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + off +(1 row) + +INSERT INTO abc VALUES ('error'); +ERROR: invalid input syntax for type integer: "error" +LINE 1: INSERT INTO abc VALUES ('error'); + ^ +ROLLBACK AND CHAIN; -- TBLOCK_ABORT_END +SHOW transaction_isolation; + transaction_isolation +----------------------- + serializable +(1 row) + +SHOW transaction_read_only; + transaction_read_only +----------------------- + off +(1 row) + +SHOW transaction_deferrable; + transaction_deferrable +------------------------ + off +(1 row) + +ROLLBACK; +SELECT * FROM abc ORDER BY 1; + a +--- + 1 + 2 + 4 + 5 +(4 rows) + +RESET default_transaction_read_only; +DROP TABLE abc; -- 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..812e40a1a3 100644 --- a/src/test/regress/sql/transactions.sql +++ b/src/test/regress/sql/transactions.sql @@ -419,6 +419,69 @@ CREATE FUNCTION create_temp_tab() RETURNS text DROP FUNCTION invert(x float8); +-- Tests for AND CHAIN + +CREATE TABLE abc (a int); + +-- set nondefault value so we have something to override below +SET default_transaction_read_only = on; + +START TRANSACTION ISOLATION LEVEL REPEATABLE READ, READ WRITE, DEFERRABLE; +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +INSERT INTO abc VALUES (1); +INSERT INTO abc VALUES (2); +COMMIT AND CHAIN; -- TBLOCK_END +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +INSERT INTO abc VALUES ('error'); +INSERT INTO abc VALUES (3); -- check it's really aborted +COMMIT AND CHAIN; -- TBLOCK_ABORT_END +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +INSERT INTO abc VALUES (4); +COMMIT; + +START TRANSACTION ISOLATION LEVEL REPEATABLE READ, READ WRITE, DEFERRABLE; +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +SAVEPOINT x; +INSERT INTO abc VALUES ('error'); +COMMIT AND CHAIN; -- TBLOCK_ABORT_PENDING +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +INSERT INTO abc VALUES (5); +COMMIT; + +-- different mix of options just for fun +START TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ WRITE, NOT DEFERRABLE; +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +INSERT INTO abc VALUES (6); +ROLLBACK AND CHAIN; -- TBLOCK_ABORT_PENDING +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +INSERT INTO abc VALUES ('error'); +ROLLBACK AND CHAIN; -- TBLOCK_ABORT_END +SHOW transaction_isolation; +SHOW transaction_read_only; +SHOW transaction_deferrable; +ROLLBACK; + +SELECT * FROM abc ORDER BY 1; + +RESET default_transaction_read_only; + +DROP TABLE abc; + + -- 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 base-commit: 1707a0d2aa6b2bcfe78f63836c769943a1a6b9e0 -- 2.20.1
From 0ee636cc22e27370eeb380a7de737e03f5c6521c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Wed, 2 Jan 2019 15:09:11 +0100 Subject: [PATCH v4 2/2] Transaction chaining support in PL/pgSQL --- doc/src/sgml/plpgsql.sgml | 9 ++++++ doc/src/sgml/spi.sgml | 14 +++++++--- src/backend/access/transam/xact.c | 4 +-- src/backend/executor/spi.c | 24 ++++++++++++++-- src/include/access/xact.h | 2 ++ src/include/executor/spi.h | 4 +-- src/pl/plperl/plperl.c | 4 +-- .../src/expected/plpgsql_transaction.out | 28 +++++++++++++++++++ src/pl/plpgsql/src/pl_exec.c | 10 ++++--- src/pl/plpgsql/src/pl_funcs.c | 10 +++++-- src/pl/plpgsql/src/pl_gram.y | 18 ++++++++++-- src/pl/plpgsql/src/pl_scanner.c | 2 ++ src/pl/plpgsql/src/plpgsql.h | 2 ++ .../plpgsql/src/sql/plpgsql_transaction.sql | 23 +++++++++++++++ src/pl/plpython/plpy_plpymodule.c | 4 +-- src/pl/tcl/pltcl.c | 4 +-- 16 files changed, 138 insertions(+), 24 deletions(-) diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 1f2abbb5d1..953c1246be 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -3490,6 +3490,15 @@ <title>Transaction Management</title> </programlisting> </para> + <para> + A new transaction starts out with default transaction characteristics such + as transaction isolation level. In cases where transactions are committed + in a loop, it might be desirable to start new transactions automatically + with the same characteristics as the previous one. The commands + <command>COMMIT AND CHAIN</command> and <command>ROLLBACK AND + CHAIN</command> accomplish this. + </para> + <para> Transaction control is only possible in <command>CALL</command> or <command>DO</command> invocations from the top level or nested diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml index 6f4f3bae6f..108c7624a8 100644 --- a/doc/src/sgml/spi.sgml +++ b/doc/src/sgml/spi.sgml @@ -4389,7 +4389,7 @@ <title>Transaction Management</title> <refsynopsisdiv> <synopsis> -void SPI_commit(void) +void SPI_commit(bool <parameter>chain</parameter>) </synopsis> </refsynopsisdiv> @@ -4402,7 +4402,10 @@ <title>Description</title> command <command>COMMIT</command>. After a transaction is committed, a new transaction has to be started using <function>SPI_start_transaction</function> before further database - actions can be executed. + actions can be executed. If <parameter>chain</parameter> is true, then a + new transaction is immediately started with the same transaction + characteristics as the just finished one, like with the SQL command + <command>COMMIT AND CHAIN</command>. </para> <para> @@ -4429,7 +4432,7 @@ <title>Description</title> <refsynopsisdiv> <synopsis> -void SPI_rollback(void) +void SPI_rollback(bool <parameter>chain</parameter>) </synopsis> </refsynopsisdiv> @@ -4442,7 +4445,10 @@ <title>Description</title> command <command>ROLLBACK</command>. After a transaction is rolled back, a new transaction has to be started using <function>SPI_start_transaction</function> before further database - actions can be executed. + actions can be executed. If <parameter>chain</parameter> is true, then a + new transaction is immediately started with the same transaction + characteristics as the just finished one, like with the SQL command + <command>COMMIT AND CHAIN</command>. </para> <para> diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 6af149d788..e8c2db3c2d 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -2774,7 +2774,7 @@ static int save_XactIsoLevel; static bool save_XactReadOnly; static bool save_XactDeferrable; -static void +void SaveTransactionCharacteristics(void) { save_XactIsoLevel = XactIsoLevel; @@ -2782,7 +2782,7 @@ SaveTransactionCharacteristics(void) save_XactDeferrable = XactDeferrable; } -static void +void RestoreTransactionCharacteristics(void) { XactIsoLevel = save_XactIsoLevel; diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index ad726676d8..d61a56e095 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -218,7 +218,7 @@ SPI_start_transaction(void) } void -SPI_commit(void) +SPI_commit(bool chain) { MemoryContext oldcontext = CurrentMemoryContext; @@ -250,14 +250,24 @@ SPI_commit(void) while (ActiveSnapshotSet()) PopActiveSnapshot(); + if (chain) + SaveTransactionCharacteristics(); + CommitTransactionCommand(); + + if (chain) + { + StartTransactionCommand(); + RestoreTransactionCharacteristics(); + } + MemoryContextSwitchTo(oldcontext); _SPI_current->internal_xact = false; } void -SPI_rollback(void) +SPI_rollback(bool chain) { MemoryContext oldcontext = CurrentMemoryContext; @@ -274,7 +284,17 @@ SPI_rollback(void) _SPI_current->internal_xact = true; + if (chain) + SaveTransactionCharacteristics(); + AbortCurrentTransaction(); + + if (chain) + { + StartTransactionCommand(); + RestoreTransactionCharacteristics(); + } + MemoryContextSwitchTo(oldcontext); _SPI_current->internal_xact = false; diff --git a/src/include/access/xact.h b/src/include/access/xact.h index c757146e4d..2abdbece6c 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -369,6 +369,8 @@ extern bool TransactionIdIsCurrentTransactionId(TransactionId xid); extern void CommandCounterIncrement(void); extern void ForceSyncCommit(void); extern void StartTransactionCommand(void); +extern void SaveTransactionCharacteristics(void); +extern void RestoreTransactionCharacteristics(void); extern void CommitTransactionCommand(void); extern void AbortCurrentTransaction(void); extern void BeginTransactionBlock(void); diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h index d2616968ac..6bf2acb3b5 100644 --- a/src/include/executor/spi.h +++ b/src/include/executor/spi.h @@ -159,8 +159,8 @@ extern int SPI_unregister_relation(const char *name); extern int SPI_register_trigger_data(TriggerData *tdata); extern void SPI_start_transaction(void); -extern void SPI_commit(void); -extern void SPI_rollback(void); +extern void SPI_commit(bool chain); +extern void SPI_rollback(bool chain); extern void SPICleanup(void); extern void AtEOXact_SPI(bool isCommit); diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index fe54b20903..93190d1099 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -3968,7 +3968,7 @@ plperl_spi_commit(void) { HoldPinnedPortals(); - SPI_commit(); + SPI_commit(false); SPI_start_transaction(); } PG_CATCH(); @@ -3995,7 +3995,7 @@ plperl_spi_rollback(void) { HoldPinnedPortals(); - SPI_rollback(); + SPI_rollback(false); SPI_start_transaction(); } PG_CATCH(); diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out index 6eedb215a4..ba0745326a 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out +++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out @@ -523,6 +523,34 @@ BEGIN END; $$; CALL transaction_test11(); +-- transaction chain +TRUNCATE test1; +DO LANGUAGE plpgsql $$ +BEGIN + ROLLBACK; + SET TRANSACTION ISOLATION LEVEL REPEATABLE READ; + FOR i IN 0..3 LOOP + RAISE INFO 'transaction_isolation = %', current_setting('transaction_isolation'); + INSERT INTO test1 (a) VALUES (i); + IF i % 2 = 0 THEN + COMMIT AND CHAIN; + ELSE + ROLLBACK AND CHAIN; + END IF; + END LOOP; +END +$$; +INFO: transaction_isolation = repeatable read +INFO: transaction_isolation = repeatable read +INFO: transaction_isolation = repeatable read +INFO: transaction_isolation = repeatable read +SELECT * FROM test1; + a | b +---+--- + 0 | + 2 | +(2 rows) + DROP TABLE test1; DROP TABLE test2; DROP TABLE test3; diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index f292fcad2d..0d8d08d6d8 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -4774,8 +4774,9 @@ exec_stmt_commit(PLpgSQL_execstate *estate, PLpgSQL_stmt_commit *stmt) { HoldPinnedPortals(); - SPI_commit(); - SPI_start_transaction(); + SPI_commit(stmt->chain); + if (!stmt->chain) + SPI_start_transaction(); estate->simple_eval_estate = NULL; plpgsql_create_econtext(estate); @@ -4793,8 +4794,9 @@ exec_stmt_rollback(PLpgSQL_execstate *estate, PLpgSQL_stmt_rollback *stmt) { HoldPinnedPortals(); - SPI_rollback(); - SPI_start_transaction(); + SPI_rollback(stmt->chain); + if (!stmt->chain) + SPI_start_transaction(); estate->simple_eval_estate = NULL; plpgsql_create_econtext(estate); diff --git a/src/pl/plpgsql/src/pl_funcs.c b/src/pl/plpgsql/src/pl_funcs.c index 4266e6cee7..0b0f892e2d 100644 --- a/src/pl/plpgsql/src/pl_funcs.c +++ b/src/pl/plpgsql/src/pl_funcs.c @@ -1318,14 +1318,20 @@ static void dump_commit(PLpgSQL_stmt_commit *stmt) { dump_ind(); - printf("COMMIT\n"); + if (stmt->chain) + printf("COMMIT AND CHAIN\n"); + else + printf("COMMIT\n"); } static void dump_rollback(PLpgSQL_stmt_rollback *stmt) { dump_ind(); - printf("ROLLBACK\n"); + if (stmt->chain) + printf("ROLLBACK AND CHAIN\n"); + else + printf("ROLLBACK\n"); } static void diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y index a979a5109d..924fbeb263 100644 --- a/src/pl/plpgsql/src/pl_gram.y +++ b/src/pl/plpgsql/src/pl_gram.y @@ -219,6 +219,8 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt); %type <ival> opt_scrollable %type <fetch> opt_fetch_direction +%type <ival> opt_transaction_chain + %type <keyword> unreserved_keyword @@ -252,6 +254,7 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt); %token <keyword> K_ABSOLUTE %token <keyword> K_ALIAS %token <keyword> K_ALL +%token <keyword> K_AND %token <keyword> K_ARRAY %token <keyword> K_ASSERT %token <keyword> K_BACKWARD @@ -259,6 +262,7 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt); %token <keyword> K_BY %token <keyword> K_CALL %token <keyword> K_CASE +%token <keyword> K_CHAIN %token <keyword> K_CLOSE %token <keyword> K_COLLATE %token <keyword> K_COLUMN @@ -2177,30 +2181,38 @@ stmt_null : K_NULL ';' } ; -stmt_commit : K_COMMIT ';' +stmt_commit : K_COMMIT opt_transaction_chain ';' { PLpgSQL_stmt_commit *new; new = palloc(sizeof(PLpgSQL_stmt_commit)); new->cmd_type = PLPGSQL_STMT_COMMIT; new->lineno = plpgsql_location_to_lineno(@1); + new->chain = $2; $$ = (PLpgSQL_stmt *)new; } ; -stmt_rollback : K_ROLLBACK ';' +stmt_rollback : K_ROLLBACK opt_transaction_chain ';' { PLpgSQL_stmt_rollback *new; new = palloc(sizeof(PLpgSQL_stmt_rollback)); new->cmd_type = PLPGSQL_STMT_ROLLBACK; new->lineno = plpgsql_location_to_lineno(@1); + new->chain = $2; $$ = (PLpgSQL_stmt *)new; } ; +opt_transaction_chain: + K_AND K_CHAIN { $$ = true; } + | K_AND K_NO K_CHAIN { $$ = false; } + | /* EMPTY */ { $$ = false; } + ; + stmt_set : K_SET { PLpgSQL_stmt_set *new; @@ -2455,10 +2467,12 @@ any_identifier : T_WORD unreserved_keyword : K_ABSOLUTE | K_ALIAS + | K_AND | K_ARRAY | K_ASSERT | K_BACKWARD | K_CALL + | K_CHAIN | K_CLOSE | K_COLLATE | K_COLUMN diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c index ab18946847..d4000bcaaa 100644 --- a/src/pl/plpgsql/src/pl_scanner.c +++ b/src/pl/plpgsql/src/pl_scanner.c @@ -99,10 +99,12 @@ static const int num_reserved_keywords = lengthof(reserved_keywords); static const ScanKeyword unreserved_keywords[] = { PG_KEYWORD("absolute", K_ABSOLUTE, UNRESERVED_KEYWORD) PG_KEYWORD("alias", K_ALIAS, UNRESERVED_KEYWORD) + PG_KEYWORD("and", K_AND, UNRESERVED_KEYWORD) PG_KEYWORD("array", K_ARRAY, UNRESERVED_KEYWORD) PG_KEYWORD("assert", K_ASSERT, UNRESERVED_KEYWORD) PG_KEYWORD("backward", K_BACKWARD, UNRESERVED_KEYWORD) PG_KEYWORD("call", K_CALL, UNRESERVED_KEYWORD) + PG_KEYWORD("chain", K_CHAIN, UNRESERVED_KEYWORD) PG_KEYWORD("close", K_CLOSE, UNRESERVED_KEYWORD) PG_KEYWORD("collate", K_COLLATE, UNRESERVED_KEYWORD) PG_KEYWORD("column", K_COLUMN, UNRESERVED_KEYWORD) diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 42177ccaa6..cac14e87d8 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -533,6 +533,7 @@ typedef struct PLpgSQL_stmt_commit { PLpgSQL_stmt_type cmd_type; int lineno; + bool chain; } PLpgSQL_stmt_commit; /* @@ -542,6 +543,7 @@ typedef struct PLpgSQL_stmt_rollback { PLpgSQL_stmt_type cmd_type; int lineno; + bool chain; } PLpgSQL_stmt_rollback; /* diff --git a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql index ac1361a8ce..0c137dd31d 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql @@ -445,6 +445,29 @@ CREATE PROCEDURE transaction_test11() CALL transaction_test11(); +-- transaction chain + +TRUNCATE test1; + +DO LANGUAGE plpgsql $$ +BEGIN + ROLLBACK; + SET TRANSACTION ISOLATION LEVEL REPEATABLE READ; + FOR i IN 0..3 LOOP + RAISE INFO 'transaction_isolation = %', current_setting('transaction_isolation'); + INSERT INTO test1 (a) VALUES (i); + IF i % 2 = 0 THEN + COMMIT AND CHAIN; + ELSE + ROLLBACK AND CHAIN; + END IF; + END LOOP; +END +$$; + +SELECT * FROM test1; + + DROP TABLE test1; DROP TABLE test2; DROP TABLE test3; diff --git a/src/pl/plpython/plpy_plpymodule.c b/src/pl/plpython/plpy_plpymodule.c index 23e49e4b75..729f402ea1 100644 --- a/src/pl/plpython/plpy_plpymodule.c +++ b/src/pl/plpython/plpy_plpymodule.c @@ -590,7 +590,7 @@ PLy_commit(PyObject *self, PyObject *args) HoldPinnedPortals(); - SPI_commit(); + SPI_commit(false); SPI_start_transaction(); /* was cleared at transaction end, reset pointer */ @@ -606,7 +606,7 @@ PLy_rollback(PyObject *self, PyObject *args) HoldPinnedPortals(); - SPI_rollback(); + SPI_rollback(false); SPI_start_transaction(); /* was cleared at transaction end, reset pointer */ diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c index 3b1454f833..27fa0fa53a 100644 --- a/src/pl/tcl/pltcl.c +++ b/src/pl/tcl/pltcl.c @@ -2931,7 +2931,7 @@ pltcl_commit(ClientData cdata, Tcl_Interp *interp, PG_TRY(); { - SPI_commit(); + SPI_commit(false); SPI_start_transaction(); } PG_CATCH(); @@ -2971,7 +2971,7 @@ pltcl_rollback(ClientData cdata, Tcl_Interp *interp, PG_TRY(); { - SPI_rollback(); + SPI_rollback(false); SPI_start_transaction(); } PG_CATCH(); -- 2.20.1