I wrote:
> Tom Lane <t...@sss.pgh.pa.us> wrote:
>
>> I agree that letting it be changed back to read/write after that
>> is surprising and unnecessary. Perhaps locking down the setting
>> at the time of first grabbing a snapshot would be appropriate.
>> IIRC that's how it works for transaction isolation level, and
>> this seems like it ought to work the same.
>
> Agreed. I can create a patch today to implement this.
Attached.
Accomplished more through mimicry (based on setting transaction
isolation level) than profound understanding of the code involved;
but it passes all regression tests on both `make check` and `make
installcheck-world`. This includes a new regression test that an
attempt to change it after a query fails. I've poked at it with
various ad hoc tests, and it is behaving as expected in those.
I wasn't too confident how to word the new failure messages.
-Kevin
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***************
*** 544,549 **** show_log_timezone(void)
--- 544,580 ----
/*
+ * 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 value, bool doit, GucSource source)
+ {
+ if (FirstSnapshotSet)
+ {
+ 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;
+ }
+ else if (IsSubTransaction())
+ {
+ ereport(GUC_complaint_elevel(source),
+ (errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
+ errmsg("read-only propery may not be changed
in a subtransaction")));
+ /* 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
*/
*** 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 ----
***************
*** 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,58 ----
-- 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;
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,50 ----
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;
+
SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY;
DROP TABLE writetest; -- fail
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
