guc_no_reset_v3.patch

application/octet-stream

Filename: guc_no_reset_v3.patch
Type: application/octet-stream
Part: 0
Message: Re: BUG #17385: "RESET transaction_isolation" inside serializable transaction causes Assert at the transaction end

Patch

Format: unified
Series: patch v3
File+
doc/src/sgml/func.sgml 4 0
src/backend/utils/misc/guc.c 17 4
src/include/utils/guc.h 2 0
src/pl/plpgsql/src/expected/plpgsql_transaction.out 0 6
src/pl/plpgsql/src/sql/plpgsql_transaction.sql 0 5
src/test/regress/expected/guc.out 1 0
src/test/regress/expected/transactions.out 34 0
src/test/regress/sql/guc.sql 1 0
src/test/regress/sql/transactions.sql 18 0
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8a802fb225..0d604e1a83 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -23881,6 +23881,10 @@ SELECT currval(pg_get_serial_sequence('sometable', 'id'));
           <literal>NO_SHOW_ALL</literal>: parameters excluded from
           <command>SHOW ALL</command> commands.
          </member>
+         <member>
+          <literal>NO_RESET</literal>: parameters do not support
+          <command>RESET</command> commands.
+         </member>
          <member>
           <literal>NO_RESET_ALL</literal>: parameters excluded from
           <command>RESET ALL</command> commands.
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index f70f7f5c01..5732526415 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1710,7 +1710,7 @@ static struct config_bool ConfigureNamesBool[] =
 		{"transaction_read_only", PGC_USERSET, CLIENT_CONN_STATEMENT,
 			gettext_noop("Sets the current transaction's read-only status."),
 			NULL,
-			GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_NO_RESET | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
 		},
 		&XactReadOnly,
 		false,
@@ -1729,7 +1729,7 @@ static struct config_bool ConfigureNamesBool[] =
 		{"transaction_deferrable", PGC_USERSET, CLIENT_CONN_STATEMENT,
 			gettext_noop("Whether to defer a read-only serializable transaction until it can be executed with no possible serialization failures."),
 			NULL,
-			GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_NO_RESET | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
 		},
 		&XactDeferrable,
 		false,
@@ -4753,7 +4753,7 @@ static struct config_enum ConfigureNamesEnum[] =
 		{"transaction_isolation", PGC_USERSET, CLIENT_CONN_STATEMENT,
 			gettext_noop("Sets the current transaction's isolation level."),
 			NULL,
-			GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_NO_RESET | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
 		},
 		&XactIsoLevel,
 		XACT_READ_COMMITTED, isolation_level_options,
@@ -7600,6 +7600,17 @@ set_config_option(const char *name, const char *value,
 		}
 	}
 
+	/* Disallow resetting and saving (and restoring) GUC_NO_RESET values */
+	if ((record->flags & GUC_NO_RESET) &&
+		(value == NULL || action == GUC_ACTION_SAVE))
+	{
+		ereport(elevel,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("parameter \"%s\" cannot be reset",
+						name)));
+		return 0;
+	}
+
 	/*
 	 * Should we set reset/stacked values?	(If so, the behavior is not
 	 * transactional.)	This is done either when we get a default value from
@@ -9723,7 +9734,7 @@ GetConfigOptionByName(const char *name, const char **varname, bool missing_ok)
 Datum
 pg_settings_get_flags(PG_FUNCTION_ARGS)
 {
-#define MAX_GUC_FLAGS	5
+#define MAX_GUC_FLAGS	6
 	char	   *varname = TextDatumGetCString(PG_GETARG_DATUM(0));
 	struct config_generic *record;
 	int			cnt = 0;
@@ -9738,6 +9749,8 @@ pg_settings_get_flags(PG_FUNCTION_ARGS)
 
 	if (record->flags & GUC_EXPLAIN)
 		flags[cnt++] = CStringGetTextDatum("EXPLAIN");
+	if (record->flags & GUC_NO_RESET)
+		flags[cnt++] = CStringGetTextDatum("NO_RESET");
 	if (record->flags & GUC_NO_RESET_ALL)
 		flags[cnt++] = CStringGetTextDatum("NO_RESET_ALL");
 	if (record->flags & GUC_NO_SHOW_ALL)
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index ea774968f0..18d522fdca 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -235,6 +235,8 @@ typedef enum
  */
 #define GUC_RUNTIME_COMPUTED  0x200000
 
+#define GUC_NO_RESET		  0x400000	/* not support RESET and save */
+
 #define GUC_UNIT				(GUC_UNIT_MEMORY | GUC_UNIT_TIME)
 
 
diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
index 254e5b7a70..759daf69be 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
@@ -576,16 +576,10 @@ BEGIN
     PERFORM 1;
     RAISE INFO '%', current_setting('transaction_isolation');
     COMMIT;
-    SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
-    RESET TRANSACTION ISOLATION LEVEL;
-    PERFORM 1;
-    RAISE INFO '%', current_setting('transaction_isolation');
-    COMMIT;
 END;
 $$;
 INFO:  read committed
 INFO:  repeatable read
-INFO:  read committed
 -- error cases
 DO LANGUAGE plpgsql $$
 BEGIN
diff --git a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
index 8d76d00daa..f8efdb01b6 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
@@ -481,11 +481,6 @@ BEGIN
     PERFORM 1;
     RAISE INFO '%', current_setting('transaction_isolation');
     COMMIT;
-    SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
-    RESET TRANSACTION ISOLATION LEVEL;
-    PERFORM 1;
-    RAISE INFO '%', current_setting('transaction_isolation');
-    COMMIT;
 END;
 $$;
 
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 3de6404ba5..801d84bd39 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -839,6 +839,7 @@ SELECT pg_settings_get_flags('does_not_exist');
 
 CREATE TABLE tab_settings_flags AS SELECT name, category,
     'EXPLAIN'          = ANY(flags) AS explain,
+    'NO_RESET'         = ANY(flags) AS no_reset,
     'NO_RESET_ALL'     = ANY(flags) AS no_reset_all,
     'NO_SHOW_ALL'      = ANY(flags) AS no_show_all,
     'NOT_IN_SAMPLE'    = ANY(flags) AS not_in_sample,
diff --git a/src/test/regress/expected/transactions.out b/src/test/regress/expected/transactions.out
index 599d511a67..01af8a1e01 100644
--- a/src/test/regress/expected/transactions.out
+++ b/src/test/regress/expected/transactions.out
@@ -44,6 +44,40 @@ SELECT * FROM xacttest;
  777 | 777.777
 (5 rows)
 
+-- Test that transaction characteristics cannot reset.
+BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
+SELECT count(*) FROM xacttest;
+ count 
+-------
+     5
+(1 row)
+
+RESET transaction_isolation; -- error
+ERROR:  parameter "transaction_isolation" cannot be reset
+END;
+BEGIN TRANSACTION READ ONLY;
+SELECT count(*) FROM xacttest;
+ count 
+-------
+     5
+(1 row)
+
+RESET transaction_read_only; -- error
+ERROR:  parameter "transaction_read_only" cannot be reset
+END;
+BEGIN TRANSACTION DEFERRABLE;
+SELECT count(*) FROM xacttest;
+ count 
+-------
+     5
+(1 row)
+
+RESET transaction_deferrable; -- error
+ERROR:  parameter "transaction_deferrable" cannot be reset
+END;
+CREATE FUNCTION errfunc() RETURNS int LANGUAGE SQL AS 'SELECT 1'
+SET transaction_read_only = on; -- error
+ERROR:  parameter "transaction_read_only" cannot be reset
 -- Read-only tests
 CREATE TABLE writetest (a int);
 CREATE TEMPORARY TABLE temptest (a int);
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index d5db101e48..c8b612b5d5 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -324,6 +324,7 @@ SELECT pg_settings_get_flags(NULL);
 SELECT pg_settings_get_flags('does_not_exist');
 CREATE TABLE tab_settings_flags AS SELECT name, category,
     'EXPLAIN'          = ANY(flags) AS explain,
+    'NO_RESET'         = ANY(flags) AS no_reset,
     'NO_RESET_ALL'     = ANY(flags) AS no_reset_all,
     'NO_SHOW_ALL'      = ANY(flags) AS no_show_all,
     'NOT_IN_SAMPLE'    = ANY(flags) AS not_in_sample,
diff --git a/src/test/regress/sql/transactions.sql b/src/test/regress/sql/transactions.sql
index 0a716b506b..144ef09356 100644
--- a/src/test/regress/sql/transactions.sql
+++ b/src/test/regress/sql/transactions.sql
@@ -35,6 +35,24 @@ SELECT oid FROM pg_class WHERE relname = 'disappear';
 -- should have members again
 SELECT * FROM xacttest;
 
+-- Test that transaction characteristics cannot reset.
+BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
+SELECT count(*) FROM xacttest;
+RESET transaction_isolation; -- error
+END;
+
+BEGIN TRANSACTION READ ONLY;
+SELECT count(*) FROM xacttest;
+RESET transaction_read_only; -- error
+END;
+
+BEGIN TRANSACTION DEFERRABLE;
+SELECT count(*) FROM xacttest;
+RESET transaction_deferrable; -- error
+END;
+
+CREATE FUNCTION errfunc() RETURNS int LANGUAGE SQL AS 'SELECT 1'
+SET transaction_read_only = on; -- error
 
 -- Read-only tests