Fix behavior of AND CHAIN outside of explicit transaction blocks

When using COMMIT AND CHAIN or ROLLBACK AND CHAIN not in an explicit
transaction block, the previous implementation would leave a
transaction block active in the ROLLBACK case but not the COMMIT case.
To fix for now, error out when using these commands not in an explicit
transaction block.  This restriction could be lifted if a sensible
definition and implementation is found.

Bug: #15977
Author: fn ln <[email protected]>
Reviewed-by: Fabien COELHO <[email protected]>

diff --git a/doc/src/sgml/ref/commit.sgml b/doc/src/sgml/ref/commit.sgml
index e4169cd2c6220e958fef58ffe03f83d56de36045..5f244cdd3c1d78324cc585c1e4442cd38bacfec1 100644 (file)
--- a/doc/src/sgml/ref/commit.sgml
+++ b/doc/src/sgml/ref/commit.sgml
@@ -77,7 +77,8 @@ COMMIT [ WORK | TRANSACTION ] [ AND [ NO ] CHAIN ]
 
   <para>
    Issuing <command>COMMIT</command> when not inside a transaction does
-   no harm, but it will provoke a warning message.
+   no harm, but it will provoke a warning message.  <command>COMMIT AND
+   CHAIN</command> when not inside a transaction is an error.
   </para>
  </refsect1>
 

diff --git a/doc/src/sgml/ref/rollback.sgml b/doc/src/sgml/ref/rollback.sgml
index a5bbf25221c9d16b8f7937cce64fa4e9914f6b08..1357eaa8323ace8b6bb66654b6b9e4df4a425832 100644 (file)
--- a/doc/src/sgml/ref/rollback.sgml
+++ b/doc/src/sgml/ref/rollback.sgml
@@ -76,7 +76,8 @@ ROLLBACK [ WORK | TRANSACTION ] [ AND [ NO ] CHAIN ]
 
   <para>
    Issuing <command>ROLLBACK</command> outside of a transaction
-   block emits a warning and otherwise has no effect.
+   block emits a warning and otherwise has no effect.  <command>ROLLBACK AND
+   CHAIN</command> outside of a transaction block is an error.
   </para>
  </refsect1>
 

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index f594d33e7a77e89b515b2881ad2d71ffbc15b949..9162286c98a658771aa3bef8bfb53f33160ca9e8 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -3721,13 +3721,21 @@ EndTransactionBlock(bool chain)
            break;
 
            /*
-            * In an implicit transaction block, commit, but issue a warning
+            * We are in an implicit transaction block.  If AND CHAIN was
+            * specified, error.  Otherwise 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")));
+           if (chain)
+               ereport(ERROR,
+                       (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
+                        /* translator: %s represents an SQL statement name */
+                        errmsg("%s can only be used in transaction blocks",
+                               "COMMIT AND CHAIN")));
+           else
+               ereport(WARNING,
+                       (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
+                        errmsg("there is no transaction in progress")));
            s->blockState = TBLOCK_END;
            result = true;
            break;
@@ -3789,15 +3797,24 @@ EndTransactionBlock(bool chain)
            break;
 
            /*
-            * The user issued COMMIT when not inside a transaction.  Issue a
-            * WARNING, staying in TBLOCK_STARTED state.  The upcoming call to
+            * The user issued COMMIT when not inside a transaction.  For
+            * COMMIT without CHAIN, issue a WARNING, staying in
+            * TBLOCK_STARTED state.  The upcoming call to
             * CommitTransactionCommand() will then close the transaction and
-            * put us back into the default state.
+            * put us back into the default state.  For COMMIT AND CHAIN,
+            * error.
             */
        case TBLOCK_STARTED:
-           ereport(WARNING,
-                   (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
-                    errmsg("there is no transaction in progress")));
+           if (chain)
+               ereport(ERROR,
+                       (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
+                        /* translator: %s represents an SQL statement name */
+                        errmsg("%s can only be used in transaction blocks",
+                               "COMMIT AND CHAIN")));
+           else
+               ereport(WARNING,
+                       (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
+                        errmsg("there is no transaction in progress")));
            result = true;
            break;
 
@@ -3899,10 +3916,10 @@ UserAbortTransactionBlock(bool chain)
            break;
 
            /*
-            * The user issued ABORT when not inside a transaction. Issue a
-            * WARNING and go to abort state.  The upcoming call to
-            * CommitTransactionCommand() will then put us back into the
-            * default state.
+            * The user issued ABORT when not inside a transaction.  For
+            * ROLLBACK without CHAIN, issue a WARNING and go to abort state.
+            * The upcoming call to CommitTransactionCommand() will then put
+            * us back into the default state.  For ROLLBACK AND CHAIN, error.
             *
             * We do the same thing with ABORT inside an implicit transaction,
             * although in this case we might be rolling back actual database
@@ -3911,9 +3928,16 @@ UserAbortTransactionBlock(bool chain)
             */
        case TBLOCK_STARTED:
        case TBLOCK_IMPLICIT_INPROGRESS:
-           ereport(WARNING,
-                   (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
-                    errmsg("there is no transaction in progress")));
+           if (chain)
+               ereport(ERROR,
+                       (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
+                        /* translator: %s represents an SQL statement name */
+                        errmsg("%s can only be used in transaction blocks",
+                               "ROLLBACK AND CHAIN")));
+           else
+               ereport(WARNING,
+                       (errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
+                        errmsg("there is no transaction in progress")));
            s->blockState = TBLOCK_ABORT_PENDING;
            break;
 

diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out
index 1b316cc9b8ca58eefdd8df9d7b7d3d5746f57ff2..213de4be6b0a1c79f67a860cc9ae6aabb6d486b7 100644 (file)
--- a/src/test/regress/expected/transactions.out
+++ b/src/test/regress/expected/transactions.out
@@ -839,6 +839,11 @@ SHOW transaction_deferrable;
 (1 row)
 
 ROLLBACK;
+-- not allowed outside a transaction block
+COMMIT AND CHAIN;  -- error
+ERROR:  COMMIT AND CHAIN can only be used in transaction blocks
+ROLLBACK AND CHAIN;  -- error
+ERROR:  ROLLBACK AND CHAIN can only be used in transaction blocks
 SELECT * FROM abc ORDER BY 1;
  a 
 ---
@@ -934,6 +939,79 @@ SELECT 2\; RELEASE SAVEPOINT sp\; SELECT 3;
 ERROR:  RELEASE SAVEPOINT can only be used in transaction blocks
 -- but this is OK, because the BEGIN converts it to a regular xact
 SELECT 1\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT;
+-- Tests for AND CHAIN in implicit transaction blocks
+SET TRANSACTION READ WRITE\; COMMIT AND CHAIN;  -- error
+ERROR:  COMMIT AND CHAIN can only be used in transaction blocks
+SHOW transaction_read_only;
+ transaction_read_only 
+-----------------------
+ off
+(1 row)
+
+SET TRANSACTION READ WRITE\; ROLLBACK AND CHAIN;  -- error
+ERROR:  ROLLBACK AND CHAIN can only be used in transaction blocks
+SHOW transaction_read_only;
+ transaction_read_only 
+-----------------------
+ off
+(1 row)
+
+CREATE TABLE abc (a int);
+-- COMMIT/ROLLBACK + COMMIT/ROLLBACK AND CHAIN
+INSERT INTO abc VALUES (7)\; COMMIT\; INSERT INTO abc VALUES (8)\; COMMIT AND CHAIN;  -- 7 commit, 8 error
+WARNING:  there is no transaction in progress
+ERROR:  COMMIT AND CHAIN can only be used in transaction blocks
+INSERT INTO abc VALUES (9)\; ROLLBACK\; INSERT INTO abc VALUES (10)\; ROLLBACK AND CHAIN;  -- 9 rollback, 10 error
+WARNING:  there is no transaction in progress
+ERROR:  ROLLBACK AND CHAIN can only be used in transaction blocks
+-- COMMIT/ROLLBACK AND CHAIN + COMMIT/ROLLBACK
+INSERT INTO abc VALUES (11)\; COMMIT AND CHAIN\; INSERT INTO abc VALUES (12)\; COMMIT;  -- 11 error, 12 not reached
+ERROR:  COMMIT AND CHAIN can only be used in transaction blocks
+INSERT INTO abc VALUES (13)\; ROLLBACK AND CHAIN\; INSERT INTO abc VALUES (14)\; ROLLBACK;  -- 13 error, 14 not reached
+ERROR:  ROLLBACK AND CHAIN can only be used in transaction blocks
+-- START TRANSACTION + COMMIT/ROLLBACK AND CHAIN
+START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; INSERT INTO abc VALUES (15)\; COMMIT AND CHAIN;  -- 15 ok
+SHOW transaction_isolation;  -- transaction is active at this point
+ transaction_isolation 
+-----------------------
+ repeatable read
+(1 row)
+
+COMMIT;
+START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; INSERT INTO abc VALUES (16)\; ROLLBACK AND CHAIN;  -- 16 ok
+SHOW transaction_isolation;  -- transaction is active at this point
+ transaction_isolation 
+-----------------------
+ repeatable read
+(1 row)
+
+ROLLBACK;
+-- START TRANSACTION + COMMIT/ROLLBACK + COMMIT/ROLLBACK AND CHAIN
+START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; INSERT INTO abc VALUES (17)\; COMMIT\; INSERT INTO abc VALUES (18)\; COMMIT AND CHAIN;  -- 17 commit, 18 error
+ERROR:  COMMIT AND CHAIN can only be used in transaction blocks
+SHOW transaction_isolation;  -- out of transaction block
+ transaction_isolation 
+-----------------------
+ read committed
+(1 row)
+
+START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; INSERT INTO abc VALUES (19)\; ROLLBACK\; INSERT INTO abc VALUES (20)\; ROLLBACK AND CHAIN;  -- 19 rollback, 20 error
+ERROR:  ROLLBACK AND CHAIN can only be used in transaction blocks
+SHOW transaction_isolation;  -- out of transaction block
+ transaction_isolation 
+-----------------------
+ read committed
+(1 row)
+
+SELECT * FROM abc ORDER BY 1;
+ a  
+----
+  7
+ 15
+ 17
+(3 rows)
+
+DROP TABLE abc;
 -- 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 812e40a1a3a0943a6334f195d3be67e9091bc108..ba96f0f36f9c72b862eb2957c92b92bf0293aa53 100644 (file)
--- a/src/test/regress/sql/transactions.sql
+++ b/src/test/regress/sql/transactions.sql
@@ -475,6 +475,10 @@ SHOW transaction_read_only;
 SHOW transaction_deferrable;
 ROLLBACK;
 
+-- not allowed outside a transaction block
+COMMIT AND CHAIN;  -- error
+ROLLBACK AND CHAIN;  -- error
+
 SELECT * FROM abc ORDER BY 1;
 
 RESET default_transaction_read_only;
@@ -536,6 +540,45 @@ SELECT 2\; RELEASE SAVEPOINT sp\; SELECT 3;
 SELECT 1\; BEGIN\; SAVEPOINT sp\; ROLLBACK TO SAVEPOINT sp\; COMMIT;
 
 
+-- Tests for AND CHAIN in implicit transaction blocks
+
+SET TRANSACTION READ WRITE\; COMMIT AND CHAIN;  -- error
+SHOW transaction_read_only;
+
+SET TRANSACTION READ WRITE\; ROLLBACK AND CHAIN;  -- error
+SHOW transaction_read_only;
+
+CREATE TABLE abc (a int);
+
+-- COMMIT/ROLLBACK + COMMIT/ROLLBACK AND CHAIN
+INSERT INTO abc VALUES (7)\; COMMIT\; INSERT INTO abc VALUES (8)\; COMMIT AND CHAIN;  -- 7 commit, 8 error
+INSERT INTO abc VALUES (9)\; ROLLBACK\; INSERT INTO abc VALUES (10)\; ROLLBACK AND CHAIN;  -- 9 rollback, 10 error
+
+-- COMMIT/ROLLBACK AND CHAIN + COMMIT/ROLLBACK
+INSERT INTO abc VALUES (11)\; COMMIT AND CHAIN\; INSERT INTO abc VALUES (12)\; COMMIT;  -- 11 error, 12 not reached
+INSERT INTO abc VALUES (13)\; ROLLBACK AND CHAIN\; INSERT INTO abc VALUES (14)\; ROLLBACK;  -- 13 error, 14 not reached
+
+-- START TRANSACTION + COMMIT/ROLLBACK AND CHAIN
+START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; INSERT INTO abc VALUES (15)\; COMMIT AND CHAIN;  -- 15 ok
+SHOW transaction_isolation;  -- transaction is active at this point
+COMMIT;
+
+START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; INSERT INTO abc VALUES (16)\; ROLLBACK AND CHAIN;  -- 16 ok
+SHOW transaction_isolation;  -- transaction is active at this point
+ROLLBACK;
+
+-- START TRANSACTION + COMMIT/ROLLBACK + COMMIT/ROLLBACK AND CHAIN
+START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; INSERT INTO abc VALUES (17)\; COMMIT\; INSERT INTO abc VALUES (18)\; COMMIT AND CHAIN;  -- 17 commit, 18 error
+SHOW transaction_isolation;  -- out of transaction block
+
+START TRANSACTION ISOLATION LEVEL REPEATABLE READ\; INSERT INTO abc VALUES (19)\; ROLLBACK\; INSERT INTO abc VALUES (20)\; ROLLBACK AND CHAIN;  -- 19 rollback, 20 error
+SHOW transaction_isolation;  -- out of transaction block
+
+SELECT * FROM abc ORDER BY 1;
+
+DROP TABLE abc;
+
+
 -- Test for successful cleanup of an aborted transaction at session exit.
 -- THIS MUST BE THE LAST TEST IN THIS FILE.