Tom Lane <[email protected]> wrote:
> What I suggested was to not allow read-only -> read-write state
> transitions except (1) before first snapshot in the main xact
> and (2) at subxact exit (the OVERRIDE case). That seems to
> accomplish the goal. Now it will also allow dropping down to
> read-only mid-subtransaction, but I don't think forbidding that
> case is worth extra complexity.
Attached is version 2. I think it does everything we discussed,
except that I'm not sure whether I addressed the assign_XactIsoLevel
issue you mentioned, since you mentioned a debug message and I only
see things that look like errors to me. If I did miss something,
I'll be happy to take another look if you can point me to the right
place.
-Kevin
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***************
*** 544,551 **** show_log_timezone(void)
--- 544,595 ----
/*
+ * 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)
+ {
+ /* 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;
+ }
+ /* Top level transaction can't change this after first snapshot. */
+ else if (FirstSnapshotSet && !IsSubTransaction())
+ {
+ ereport(GUC_complaint_elevel(source),
+ (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
+ errmsg("read-only property must be set before
any query")));
+ /* 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 */
+ else 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;
+ }
+
+ /*
* SET TRANSACTION ISOLATION LEVEL
*/
+ extern char *XactIsoLevel_string; /* in guc.c */
const char *
assign_XactIsoLevel(const char *value, bool doit, GucSource source)
***************
*** 559,565 **** assign_XactIsoLevel(const char *value, bool doit, GucSource
source)
if (source != PGC_S_OVERRIDE)
return NULL;
}
! else if (IsSubTransaction())
{
ereport(GUC_complaint_elevel(source),
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
--- 603,610 ----
if (source != PGC_S_OVERRIDE)
return NULL;
}
! else if (IsSubTransaction()
! && strcmp(value, XactIsoLevel_string) != 0)
{
ereport(GUC_complaint_elevel(source),
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 169,175 **** 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);
--- 169,174 ----
***************
*** 426,432 **** 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;
--- 425,430 ----
***************
*** 441,446 **** static int effective_io_concurrency;
--- 439,445 ----
/* should be static, but commands/variable.c needs to get at these */
char *role_string;
char *session_authorization_string;
+ char *XactIsoLevel_string;
/*
***************
*** 7837,7870 **** 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)
{
--- 7836,7841 ----
*** a/src/include/commands/variable.h
--- b/src/include/commands/variable.h
***************
*** 21,26 **** extern const char *show_timezone(void);
--- 21,28 ----
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,48 **** SELECT * FROM aggtest;
--- 43,92 ----
-- 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,44 **** SELECT * FROM aggtest;
--- 39,72 ----
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
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
