Robert Haas <robertmh...@gmail.com> wrote:
> Jeff Janes <jeff.ja...@gmail.com> wrote:
>> I found the following message somewhat confusing:
>> ERROR: read-only property must be set before any query
>
> I think what we need here is two messages, this one and a similar
> one that starts with "read-write property...".
Done. I started out by being cute with plugging "only" or "write"
into a single message, but then figured that might be hard on
translators; so I went with two separate messages.
Also, I noticed we seemed to be using "transaction read-only mode"
and "transaction read-write mode" elsewhere, so I made this
consistent with the others while I was at it. Hopefully that was a
good idea.
>> When a subtransaction has set the mode more stringent than the
>> top-level transaction did, that setting is reversed when the
>> subtransaction ends (whether by success or by rollback), which
>> was discussed as the desired behavior. But the included
>> regression tests do not exercise that case by testing the case
>> where a SAVEPOINT is either rolled back or released. Should
>> those tests be included?
>
> +1.
Done.
-Kevin
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***************
*** 544,572 **** show_log_timezone(void)
/*
* SET TRANSACTION ISOLATION LEVEL
*/
const char *
assign_XactIsoLevel(const char *value, bool doit, GucSource source)
{
! if (FirstSnapshotSet)
{
! 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)
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)
return NULL;
}
if (strcmp(value, "serializable") == 0)
--- 544,617 ----
/*
+ * 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(newval
+ ? "transaction
read-only mode must be set before any query"
+ : "transaction
read-write mode 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)
{
! /* 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;
! }
! /* 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,174 **** 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);
--- 168,173 ----
***************
*** 425,431 **** 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;
--- 424,429 ----
***************
*** 440,445 **** static int effective_io_concurrency;
--- 438,444 ----
/* should be static, but commands/variable.c needs to get at these */
char *role_string;
char *session_authorization_string;
+ char *XactIsoLevel_string;
/*
***************
*** 7843,7876 **** 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)
{
--- 7842,7847 ----
*** 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,123 ----
-- 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: transaction read-write mode 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;
+ BEGIN;
+ SET TRANSACTION READ WRITE; -- ok
+ SAVEPOINT x;
+ SET TRANSACTION READ ONLY; -- ok
+ SELECT * FROM writetest; -- ok
+ a
+ ---
+ (0 rows)
+
+ ROLLBACK TO SAVEPOINT x;
+ SHOW transaction_read_only; -- off
+ transaction_read_only
+ -----------------------
+ off
+ (1 row)
+
+ SAVEPOINT y;
+ SET TRANSACTION READ ONLY; -- ok
+ SELECT * FROM writetest; -- ok
+ a
+ ---
+ (0 rows)
+
+ RELEASE SAVEPOINT y;
+ SHOW transaction_read_only; -- off
+ transaction_read_only
+ -----------------------
+ off
+ (1 row)
+
+ 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,86 ----
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;
+
+ BEGIN;
+ SET TRANSACTION READ WRITE; -- ok
+ SAVEPOINT x;
+ SET TRANSACTION READ ONLY; -- ok
+ SELECT * FROM writetest; -- ok
+ ROLLBACK TO SAVEPOINT x;
+ SHOW transaction_read_only; -- off
+ SAVEPOINT y;
+ SET TRANSACTION READ ONLY; -- ok
+ SELECT * FROM writetest; -- ok
+ RELEASE SAVEPOINT y;
+ SHOW transaction_read_only; -- off
+ 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
