From 671de0c55ba20133b858933e02346327c65791a4 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@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

