--- 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