read-only-4.patch

--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -544,29 +544,72 @@ show_log_timezone(void)
 
 
 /*
+ * SET TRANSACTION READ ONLY and SET TRANSACTION READ WRITE
+ *
+ * These should be transaction properties which can be set in exactly the
+ * same points in time that transaction isolation may be set.
+ */
+bool
+assign_transaction_read_only(bool newval, bool doit, GucSource source)
+{
+	/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
+	if (source != PGC_S_OVERRIDE)
+	{
+		/* Can't go to r/w mode inside a r/o transaction */
+		if (newval == false && XactReadOnly && IsSubTransaction())
+		{
+			ereport(GUC_complaint_elevel(source),
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("cannot set transaction read-write mode inside a read-only transaction")));
+			return false;
+		}
+		/* Top level transaction can't change this after first snapshot. */
+		if (FirstSnapshotSet && !IsSubTransaction())
+		{
+			ereport(GUC_complaint_elevel(source),
+					(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
+					 errmsg("read-only property must be set before any query")));
+			return false;
+		}
+		/* Can't go to r/w mode while recovery is still active */
+		if (newval == false && XactReadOnly && RecoveryInProgress())
+		{
+			ereport(GUC_complaint_elevel(source),
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("cannot set transaction read-write mode during recovery")));
+			return false;
+		}
+	}
+
+	return true;
+}
+
+/*
  * SET TRANSACTION ISOLATION LEVEL
  */
+extern char *XactIsoLevel_string;		/* in guc.c */
 
 const char *
 assign_XactIsoLevel(const char *value, bool doit, GucSource source)
 {
-	if (FirstSnapshotSet)
+	/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
+	if (source != PGC_S_OVERRIDE)
 	{
-		ereport(GUC_complaint_elevel(source),
-				(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
-				 errmsg("SET TRANSACTION ISOLATION LEVEL must be called before any query")));
-		/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
-		if (source != PGC_S_OVERRIDE)
+		if (FirstSnapshotSet)
+		{
+			ereport(GUC_complaint_elevel(source),
+					(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
+					 errmsg("SET TRANSACTION ISOLATION LEVEL must be called before any query")));
 			return NULL;
-	}
-	else if (IsSubTransaction())
-	{
-		ereport(GUC_complaint_elevel(source),
-				(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
-				 errmsg("SET TRANSACTION ISOLATION LEVEL must not be called in a subtransaction")));
-		/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
-		if (source != PGC_S_OVERRIDE)
+		}
+		/* We ignore a subtransaction setting it to the existing value. */
+		if (IsSubTransaction() && strcmp(value, XactIsoLevel_string) != 0)
+		{
+			ereport(GUC_complaint_elevel(source),
+					(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
+					 errmsg("SET TRANSACTION ISOLATION LEVEL must not be called in a subtransaction")));
 			return NULL;
+		}
 	}
 
 	if (strcmp(value, "serializable") == 0)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -168,7 +168,6 @@ static bool assign_bonjour(bool newval, bool doit, GucSource source);
 static bool assign_ssl(bool newval, bool doit, GucSource source);
 static bool assign_stage_log_stats(bool newval, bool doit, GucSource source);
 static bool assign_log_stats(bool newval, bool doit, GucSource source);
-static bool assign_transaction_read_only(bool newval, bool doit, GucSource source);
 static const char *assign_canonical_path(const char *newval, bool doit, GucSource source);
 static const char *assign_timezone_abbreviations(const char *newval, bool doit, GucSource source);
 static const char *show_archive_command(void);
@@ -425,7 +424,6 @@ static int	server_version_num;
 static char *timezone_string;
 static char *log_timezone_string;
 static char *timezone_abbreviations_string;
-static char *XactIsoLevel_string;
 static char *custom_variable_classes;
 static int	max_function_args;
 static int	max_index_keys;
@@ -440,6 +438,7 @@ static int	effective_io_concurrency;
 /* should be static, but commands/variable.c needs to get at these */
 char	   *role_string;
 char	   *session_authorization_string;
+char	   *XactIsoLevel_string;
 
 
 /*
@@ -7843,34 +7842,6 @@ assign_log_stats(bool newval, bool doit, GucSource source)
 	return true;
 }
 
-static bool
-assign_transaction_read_only(bool newval, bool doit, GucSource source)
-{
-	/* Can't go to r/w mode inside a r/o transaction */
-	if (newval == false && XactReadOnly && IsSubTransaction())
-	{
-		ereport(GUC_complaint_elevel(source),
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("cannot set transaction read-write mode inside a read-only transaction")));
-		/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
-		if (source != PGC_S_OVERRIDE)
-			return false;
-	}
-
-	/* Can't go to r/w mode while recovery is still active */
-	if (newval == false && XactReadOnly && RecoveryInProgress())
-	{
-		ereport(GUC_complaint_elevel(source),
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-		  errmsg("cannot set transaction read-write mode during recovery")));
-		/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
-		if (source != PGC_S_OVERRIDE)
-			return false;
-	}
-
-	return true;
-}
-
 static const char *
 assign_canonical_path(const char *newval, bool doit, GucSource source)
 {
--- a/src/include/commands/variable.h
+++ b/src/include/commands/variable.h
@@ -21,6 +21,8 @@ extern const char *show_timezone(void);
 extern const char *assign_log_timezone(const char *value,
 					bool doit, GucSource source);
 extern const char *show_log_timezone(void);
+extern bool assign_transaction_read_only(bool value,
+					bool doit, GucSource source);
 extern const char *assign_XactIsoLevel(const char *value,
 					bool doit, GucSource source);
 extern const char *show_XactIsoLevel(void);
--- a/src/test/regress/expected/transactions.out
+++ b/src/test/regress/expected/transactions.out
@@ -43,6 +43,50 @@ SELECT * FROM aggtest;
 -- Read-only tests
 CREATE TABLE writetest (a int);
 CREATE TEMPORARY TABLE temptest (a int);
+BEGIN;
+SET TRANSACTION READ ONLY; -- ok
+SELECT * FROM writetest; -- ok
+ a 
+---
+(0 rows)
+
+SET TRANSACTION READ WRITE; --fail
+ERROR:  read-only property must be set before any query
+COMMIT;
+BEGIN;
+SET TRANSACTION READ ONLY; -- ok
+SET TRANSACTION READ WRITE; -- ok
+SET TRANSACTION READ ONLY; -- ok
+SELECT * FROM writetest; -- ok
+ a 
+---
+(0 rows)
+
+SAVEPOINT x;
+SET TRANSACTION READ ONLY; -- ok
+SELECT * FROM writetest; -- ok
+ a 
+---
+(0 rows)
+
+SET TRANSACTION READ ONLY; -- ok
+SET TRANSACTION READ WRITE; --fail
+ERROR:  cannot set transaction read-write mode inside a read-only transaction
+COMMIT;
+BEGIN;
+SET TRANSACTION READ WRITE; -- ok
+SAVEPOINT x;
+SET TRANSACTION READ WRITE; -- ok
+SET TRANSACTION READ ONLY; -- ok
+SELECT * FROM writetest; -- ok
+ a 
+---
+(0 rows)
+
+SET TRANSACTION READ ONLY; -- ok
+SET TRANSACTION READ WRITE; --fail
+ERROR:  cannot set transaction read-write mode inside a read-only transaction
+COMMIT;
 SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY;
 DROP TABLE writetest; -- fail
 ERROR:  cannot execute DROP TABLE in a read-only transaction
--- a/src/test/regress/sql/transactions.sql
+++ b/src/test/regress/sql/transactions.sql
@@ -39,6 +39,34 @@ SELECT * FROM aggtest;
 CREATE TABLE writetest (a int);
 CREATE TEMPORARY TABLE temptest (a int);
 
+BEGIN;
+SET TRANSACTION READ ONLY; -- ok
+SELECT * FROM writetest; -- ok
+SET TRANSACTION READ WRITE; --fail
+COMMIT;
+
+BEGIN;
+SET TRANSACTION READ ONLY; -- ok
+SET TRANSACTION READ WRITE; -- ok
+SET TRANSACTION READ ONLY; -- ok
+SELECT * FROM writetest; -- ok
+SAVEPOINT x;
+SET TRANSACTION READ ONLY; -- ok
+SELECT * FROM writetest; -- ok
+SET TRANSACTION READ ONLY; -- ok
+SET TRANSACTION READ WRITE; --fail
+COMMIT;
+
+BEGIN;
+SET TRANSACTION READ WRITE; -- ok
+SAVEPOINT x;
+SET TRANSACTION READ WRITE; -- ok
+SET TRANSACTION READ ONLY; -- ok
+SELECT * FROM writetest; -- ok
+SET TRANSACTION READ ONLY; -- ok
+SET TRANSACTION READ WRITE; --fail
+COMMIT;
+
 SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY;
 
 DROP TABLE writetest; -- fail