Skip to content

Commit 6f59777

Browse files
robertmhaasrobertmhaas
committed
Code cleanup for assign_transaction_read_only.
As in commit fb4c5d2 on 2011-01-21, this avoids spurious debug messages and allows idempotent changes at any time. Along the way, make assign_XactIsoLevel allow idempotent changes even when not within a subtransaction, to be consistent with the new coding of assign_transaction_read_only and because there's no compelling reason to do otherwise. Kevin Grittner, with some adjustments.
1 parent cc73c16 commit 6f59777

File tree

5 files changed

+170
-31
lines changed

5 files changed

+170
-31
lines changed

‎src/backend/commands/variable.c‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,14 +543,63 @@ show_log_timezone(void)
543543
}
544544

545545

546+
/*
547+
* SET TRANSACTION READ ONLY and SET TRANSACTION READ WRITE
548+
*
549+
* We allow idempotent changes (r/w -> r/w and r/o -> r/o) at any time, and
550+
* we also always allow changes from read-write to read-only. However,
551+
* read-only to read-write may be changed only when source == PGC_S_OVERRIDE
552+
* (i.e. we're aborting a read only transaction and restoring the previous
553+
* setting) or in a top-level transaction that has not yet taken an initial
554+
* snapshot.
555+
*/
556+
bool
557+
assign_transaction_read_only(bool newval, bool doit, GucSource source)
558+
{
559+
if (source != PGC_S_OVERRIDE && newval == false && XactReadOnly)
560+
{
561+
/* Can't go to r/w mode inside a r/o transaction */
562+
if (IsSubTransaction())
563+
{
564+
ereport(GUC_complaint_elevel(source),
565+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
566+
errmsg("cannot set transaction read-write mode inside a read-only transaction")));
567+
return false;
568+
}
569+
/* Top level transaction can't change to r/w after first snapshot. */
570+
if (FirstSnapshotSet)
571+
{
572+
ereport(GUC_complaint_elevel(source),
573+
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),
574+
errmsg("transaction read-write mode must be set before any query")));
575+
return false;
576+
}
577+
/* Can't go to r/w mode while recovery is still active */
578+
if (RecoveryInProgress())
579+
{
580+
ereport(GUC_complaint_elevel(source),
581+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
582+
errmsg("cannot set transaction read-write mode during recovery")));
583+
return false;
584+
}
585+
}
586+
587+
return true;
588+
}
589+
546590
/*
547591
* SET TRANSACTION ISOLATION LEVEL
592+
*
593+
* We allow idempotent changes at any time, but otherwise this can only be
594+
* changed from a toplevel transaction that has not yet taken a snapshot, or
595+
* when source == PGC_S_OVERRIDE (i.e. we're aborting a transaction and
596+
* restoring the previously set value).
548597
*/
549598
const char *
550599
assign_XactIsoLevel(const char *value, bool doit, GucSource source)
551600
{
552601
/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
553-
if (source != PGC_S_OVERRIDE)
602+
if (source != PGC_S_OVERRIDE && strcmp(value, XactIsoLevel_string) != 0)
554603
{
555604
if (FirstSnapshotSet)
556605
{
@@ -560,7 +609,7 @@ assign_XactIsoLevel(const char *value, bool doit, GucSource source)
560609
return NULL;
561610
}
562611
/* We ignore a subtransaction setting it to the existing value. */
563-
if (IsSubTransaction() && strcmp(value, XactIsoLevel_string) != 0)
612+
if (IsSubTransaction())
564613
{
565614
ereport(GUC_complaint_elevel(source),
566615
(errcode(ERRCODE_ACTIVE_SQL_TRANSACTION),

‎src/backend/utils/misc/guc.c‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,6 @@ static bool assign_bonjour(bool newval, bool doit, GucSource source);
168168
static bool assign_ssl(bool newval, bool doit, GucSource source);
169169
static bool assign_stage_log_stats(bool newval, bool doit, GucSource source);
170170
static bool assign_log_stats(bool newval, bool doit, GucSource source);
171-
static bool assign_transaction_read_only(bool newval, bool doit, GucSource source);
172171
static const char *assign_canonical_path(const char *newval, bool doit, GucSource source);
173172
static const char *assign_timezone_abbreviations(const char *newval, bool doit, GucSource source);
174173
static const char *show_archive_command(void);
@@ -7843,34 +7842,6 @@ assign_log_stats(bool newval, bool doit, GucSource source)
78437842
return true;
78447843
}
78457844

7846-
static bool
7847-
assign_transaction_read_only(bool newval, bool doit, GucSource source)
7848-
{
7849-
/* Can't go to r/w mode inside a r/o transaction */
7850-
if (newval == false && XactReadOnly && IsSubTransaction())
7851-
{
7852-
ereport(GUC_complaint_elevel(source),
7853-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
7854-
errmsg("cannot set transaction read-write mode inside a read-only transaction")));
7855-
/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
7856-
if (source != PGC_S_OVERRIDE)
7857-
return false;
7858-
}
7859-
7860-
/* Can't go to r/w mode while recovery is still active */
7861-
if (newval == false && XactReadOnly && RecoveryInProgress())
7862-
{
7863-
ereport(GUC_complaint_elevel(source),
7864-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
7865-
errmsg("cannot set transaction read-write mode during recovery")));
7866-
/* source == PGC_S_OVERRIDE means do it anyway, eg at xact abort */
7867-
if (source != PGC_S_OVERRIDE)
7868-
return false;
7869-
}
7870-
7871-
return true;
7872-
}
7873-
78747845
static const char *
78757846
assign_canonical_path(const char *newval, bool doit, GucSource source)
78767847
{

‎src/include/commands/variable.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ extern const char *show_timezone(void);
2121
extern const char *assign_log_timezone(const char *value,
2222
bool doit, GucSource source);
2323
extern const char *show_log_timezone(void);
24+
extern bool assign_transaction_read_only(bool value,
25+
bool doit, GucSource source);
2426
extern const char *assign_XactIsoLevel(const char *value,
2527
bool doit, GucSource source);
2628
extern const char *show_XactIsoLevel(void);

‎src/test/regress/expected/transactions.out‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,81 @@ SELECT * FROM aggtest;
4343
-- Read-only tests
4444
CREATE TABLE writetest (a int);
4545
CREATE TEMPORARY TABLE temptest (a int);
46+
BEGIN;
47+
SET TRANSACTION READ ONLY; -- ok
48+
SELECT * FROM writetest; -- ok
49+
a
50+
---
51+
(0 rows)
52+
53+
SET TRANSACTION READ WRITE; --fail
54+
ERROR: transaction read-write mode must be set before any query
55+
COMMIT;
56+
BEGIN;
57+
SET TRANSACTION READ ONLY; -- ok
58+
SET TRANSACTION READ WRITE; -- ok
59+
SET TRANSACTION READ ONLY; -- ok
60+
SELECT * FROM writetest; -- ok
61+
a
62+
---
63+
(0 rows)
64+
65+
SAVEPOINT x;
66+
SET TRANSACTION READ ONLY; -- ok
67+
SELECT * FROM writetest; -- ok
68+
a
69+
---
70+
(0 rows)
71+
72+
SET TRANSACTION READ ONLY; -- ok
73+
SET TRANSACTION READ WRITE; --fail
74+
ERROR: cannot set transaction read-write mode inside a read-only transaction
75+
COMMIT;
76+
BEGIN;
77+
SET TRANSACTION READ WRITE; -- ok
78+
SAVEPOINT x;
79+
SET TRANSACTION READ WRITE; -- ok
80+
SET TRANSACTION READ ONLY; -- ok
81+
SELECT * FROM writetest; -- ok
82+
a
83+
---
84+
(0 rows)
85+
86+
SET TRANSACTION READ ONLY; -- ok
87+
SET TRANSACTION READ WRITE; --fail
88+
ERROR: cannot set transaction read-write mode inside a read-only transaction
89+
COMMIT;
90+
BEGIN;
91+
SET TRANSACTION READ WRITE; -- ok
92+
SAVEPOINT x;
93+
SET TRANSACTION READ ONLY; -- ok
94+
SELECT * FROM writetest; -- ok
95+
a
96+
---
97+
(0 rows)
98+
99+
ROLLBACK TO SAVEPOINT x;
100+
SHOW transaction_read_only; -- off
101+
transaction_read_only
102+
-----------------------
103+
off
104+
(1 row)
105+
106+
SAVEPOINT y;
107+
SET TRANSACTION READ ONLY; -- ok
108+
SELECT * FROM writetest; -- ok
109+
a
110+
---
111+
(0 rows)
112+
113+
RELEASE SAVEPOINT y;
114+
SHOW transaction_read_only; -- off
115+
transaction_read_only
116+
-----------------------
117+
off
118+
(1 row)
119+
120+
COMMIT;
46121
SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY;
47122
DROP TABLE writetest; -- fail
48123
ERROR: cannot execute DROP TABLE in a read-only transaction

‎src/test/regress/sql/transactions.sql‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,48 @@ SELECT * FROM aggtest;
3939
CREATE TABLE writetest (a int);
4040
CREATE TEMPORARY TABLE temptest (a int);
4141

42+
BEGIN;
43+
SET TRANSACTION READ ONLY; -- ok
44+
SELECT * FROM writetest; -- ok
45+
SET TRANSACTION READ WRITE; --fail
46+
COMMIT;
47+
48+
BEGIN;
49+
SET TRANSACTION READ ONLY; -- ok
50+
SET TRANSACTION READ WRITE; -- ok
51+
SET TRANSACTION READ ONLY; -- ok
52+
SELECT * FROM writetest; -- ok
53+
SAVEPOINT x;
54+
SET TRANSACTION READ ONLY; -- ok
55+
SELECT * FROM writetest; -- ok
56+
SET TRANSACTION READ ONLY; -- ok
57+
SET TRANSACTION READ WRITE; --fail
58+
COMMIT;
59+
60+
BEGIN;
61+
SET TRANSACTION READ WRITE; -- ok
62+
SAVEPOINT x;
63+
SET TRANSACTION READ WRITE; -- ok
64+
SET TRANSACTION READ ONLY; -- ok
65+
SELECT * FROM writetest; -- ok
66+
SET TRANSACTION READ ONLY; -- ok
67+
SET TRANSACTION READ WRITE; --fail
68+
COMMIT;
69+
70+
BEGIN;
71+
SET TRANSACTION READ WRITE; -- ok
72+
SAVEPOINT x;
73+
SET TRANSACTION READ ONLY; -- ok
74+
SELECT * FROM writetest; -- ok
75+
ROLLBACK TO SAVEPOINT x;
76+
SHOW transaction_read_only; -- off
77+
SAVEPOINT y;
78+
SET TRANSACTION READ ONLY; -- ok
79+
SELECT * FROM writetest; -- ok
80+
RELEASE SAVEPOINT y;
81+
SHOW transaction_read_only; -- off
82+
COMMIT;
83+
4284
SET SESSION CHARACTERISTICS AS TRANSACTION READ ONLY;
4385

4486
DROP TABLE writetest; -- fail

0 commit comments

Comments
 (0)