rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Introduce GUC_NO_RESET flag. 

Previously, the transaction-property GUCs such as transaction_isolation
could be reset after starting a transaction, because we marked them
as GUC_NO_RESET_ALL but still allowed a targeted RESET.  That leads to
assertion failures or worse, because those properties aren't supposed
to change after we've acquired a transaction snapshot.

There are some NO_RESET_ALL variables for which RESET is okay, so
we can't just redefine the semantics of that flag.  Instead introduce
a separate GUC_NO_RESET flag.  Mark "seed", as well as the transaction
property GUCs, as GUC_NO_RESET.

We have to disallow GUC_ACTION_SAVE as well as straight RESET, because
otherwise a function having a "SET transaction_isolation" clause can
still break things: the end-of-function restore action is equivalent
to a RESET.

No back-patch, as it's conceivable that someone is doing something
this patch will forbid (like resetting one of these GUCs at transaction
start, or "CREATE FUNCTION ... SET transaction_read_only = 1") and not
running into problems with it today.  Given how long we've had this
issue and not noticed, the side effects in non-assert builds can't be
too serious.

Per bug #17385 from Andrew Bille.

Masahiko Sawada

Discussion: https://postgr.es/m/17385-9ee529fb091f0ce5@postgresql.org
This commit is contained in:
Tom Lane 2022-09-27 11:47:12 -04:00
parent 4148c8b3da
commit 3853664265
11 changed files with 103 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
doc/src/sgml/func.sgml
View File

@ -24353,6 +24353,12 @@ SELECT collation for ('foo' COLLATE "de_DE");
<command>SHOW ALL</command> commands.
</entry>
</row>
<row>
<entry><literal>NO_RESET</literal></entry>
<entry>Parameters with this flag do not support
<command>RESET</command> commands.
</entry>
</row>
<row>
<entry><literal>NO_RESET_ALL</literal></entry>
<entry>Parameters with this flag are excluded from

20
src/backend/utils/misc/guc.c
View File

@ -3243,6 +3243,26 @@ set_config_option_ext(const char *name, const char *value,
}
}
/* Disallow resetting and saving GUC_NO_RESET values */
if (record->flags & GUC_NO_RESET)
{
if (value == NULL)
{
ereport(elevel,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("parameter \"%s\" cannot be reset", name)));
return 0;
}
if (action == GUC_ACTION_SAVE)
{
ereport(elevel,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("parameter \"%s\" cannot be set locally in functions",
name)));
return 0;
}
}
/*
* Should we set reset/stacked values? (If so, the behavior is not
* transactional.) This is done either when we get a default value from

7
src/backend/utils/misc/guc_funcs.c
View File

@ -141,9 +141,6 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
WarnNoTransactionBlock(isTopLevel, "SET LOCAL");
/* fall through */
case VAR_RESET:
if (strcmp(stmt->name, "transaction_isolation") == 0)
WarnNoTransactionBlock(isTopLevel, "RESET TRANSACTION");
(void) set_config_option(stmt->name,
NULL,
(superuser() ? PGC_SUSET : PGC_USERSET),
@ -539,7 +536,7 @@ ShowAllGUCConfig(DestReceiver *dest)
Datum
pg_settings_get_flags(PG_FUNCTION_ARGS)
{
#define MAX_GUC_FLAGS 5
#define MAX_GUC_FLAGS 6
char *varname = TextDatumGetCString(PG_GETARG_DATUM(0));
struct config_generic *record;
int cnt = 0;
@ -554,6 +551,8 @@ pg_settings_get_flags(PG_FUNCTION_ARGS)
if (record->flags & GUC_EXPLAIN)
flags[cnt++] = CStringGetTextDatum("EXPLAIN");
if (record->flags & GUC_NO_RESET)
flags[cnt++] = CStringGetTextDatum("NO_RESET");
if (record->flags & GUC_NO_RESET_ALL)
flags[cnt++] = CStringGetTextDatum("NO_RESET_ALL");
if (record->flags & GUC_NO_SHOW_ALL)

8
src/backend/utils/misc/guc_tables.c
View File

@ -1505,7 +1505,7 @@ struct config_bool ConfigureNamesBool[] =
{"transaction_read_only", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Sets the current transaction's read-only status."),
NULL,
GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
GUC_NO_RESET | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
},
&XactReadOnly,
false,
@ -1524,7 +1524,7 @@ struct config_bool ConfigureNamesBool[] =
{"transaction_deferrable", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Whether to defer a read-only serializable transaction until it can be executed with no possible serialization failures."),
NULL,
GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
GUC_NO_RESET | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
},
&XactDeferrable,
false,
@ -3606,7 +3606,7 @@ struct config_real ConfigureNamesReal[] =
{"seed", PGC_USERSET, UNGROUPED,
gettext_noop("Sets the seed for random-number generation."),
NULL,
GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
GUC_NO_SHOW_ALL | GUC_NO_RESET | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
},
&phony_random_seed,
0.0, -1.0, 1.0,
@ -4557,7 +4557,7 @@ struct config_enum ConfigureNamesEnum[] =
{"transaction_isolation", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Sets the current transaction's isolation level."),
NULL,
GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
GUC_NO_RESET | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
},
&XactIsoLevel,
XACT_READ_COMMITTED, isolation_level_options,

1
src/include/utils/guc.h
View File

@ -207,6 +207,7 @@ typedef enum
#define GUC_LIST_INPUT 0x0001 /* input can be list format */
#define GUC_LIST_QUOTE 0x0002 /* double-quote list elements */
#define GUC_NO_SHOW_ALL 0x0004 /* exclude from SHOW ALL */
#define GUC_NO_RESET 0x400000 /* disallow RESET and SAVE */
#define GUC_NO_RESET_ALL 0x0008 /* exclude from RESET ALL */
#define GUC_REPORT 0x0010 /* auto-report changes to client */
#define GUC_NOT_IN_SAMPLE 0x0020 /* not in postgresql.conf.sample */

5
src/pl/plpgsql/src/expected/plpgsql_transaction.out
View File

@ -576,8 +576,7 @@ BEGIN
PERFORM 1;
RAISE INFO '%', current_setting('transaction_isolation');
COMMIT;
SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
RESET TRANSACTION ISOLATION LEVEL;
SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
PERFORM 1;
RAISE INFO '%', current_setting('transaction_isolation');
COMMIT;
@ -585,7 +584,7 @@ END;
$$;
INFO: read committed
INFO: repeatable read
INFO: read committed
INFO: serializable
-- error cases
DO LANGUAGE plpgsql $$
BEGIN

3
src/pl/plpgsql/src/sql/plpgsql_transaction.sql
View File

@ -481,8 +481,7 @@ BEGIN
PERFORM 1;
RAISE INFO '%', current_setting('transaction_isolation');
COMMIT;
SET TRANSACTION ISOLATION LEVEL REPEATABLE READ;
RESET TRANSACTION ISOLATION LEVEL;
SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
PERFORM 1;
RAISE INFO '%', current_setting('transaction_isolation');
COMMIT;

9
src/test/regress/expected/guc.out
View File

@ -839,6 +839,7 @@ SELECT pg_settings_get_flags('does_not_exist');
CREATE TABLE tab_settings_flags AS SELECT name, category,
'EXPLAIN' = ANY(flags) AS explain,
'NO_RESET' = ANY(flags) AS no_reset,
'NO_RESET_ALL' = ANY(flags) AS no_reset_all,
'NO_SHOW_ALL' = ANY(flags) AS no_show_all,
'NOT_IN_SAMPLE' = ANY(flags) AS not_in_sample,
@ -906,4 +907,12 @@ SELECT name FROM tab_settings_flags
------
(0 rows)
-- NO_RESET implies NO_RESET_ALL.
SELECT name FROM tab_settings_flags
WHERE no_reset AND NOT no_reset_all
ORDER BY 1;
name
------
(0 rows)
DROP TABLE tab_settings_flags;

34
src/test/regress/expected/transactions.out
View File

@ -44,6 +44,40 @@ SELECT * FROM xacttest;
777 | 777.777
(5 rows)
-- Test that transaction characteristics cannot be reset.
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
SELECT COUNT(*) FROM xacttest;
count
-------
5
(1 row)
RESET transaction_isolation; -- error
ERROR: parameter "transaction_isolation" cannot be reset
END;
BEGIN TRANSACTION READ ONLY;
SELECT COUNT(*) FROM xacttest;
count
-------
5
(1 row)
RESET transaction_read_only; -- error
ERROR: parameter "transaction_read_only" cannot be reset
END;
BEGIN TRANSACTION DEFERRABLE;
SELECT COUNT(*) FROM xacttest;
count
-------
5
(1 row)
RESET transaction_deferrable; -- error
ERROR: parameter "transaction_deferrable" cannot be reset
END;
CREATE FUNCTION errfunc() RETURNS int LANGUAGE SQL AS 'SELECT 1'
SET transaction_read_only = on; -- error
ERROR: parameter "transaction_read_only" cannot be set locally in functions
-- Read-only tests
CREATE TABLE writetest (a int);
CREATE TEMPORARY TABLE temptest (a int);

5
src/test/regress/sql/guc.sql
View File

@ -324,6 +324,7 @@ SELECT pg_settings_get_flags(NULL);
SELECT pg_settings_get_flags('does_not_exist');
CREATE TABLE tab_settings_flags AS SELECT name, category,
'EXPLAIN' = ANY(flags) AS explain,
'NO_RESET' = ANY(flags) AS no_reset,
'NO_RESET_ALL' = ANY(flags) AS no_reset_all,
'NO_SHOW_ALL' = ANY(flags) AS no_show_all,
'NOT_IN_SAMPLE' = ANY(flags) AS not_in_sample,
@ -360,4 +361,8 @@ SELECT name FROM tab_settings_flags
SELECT name FROM tab_settings_flags
WHERE no_show_all AND NOT not_in_sample
ORDER BY 1;
-- NO_RESET implies NO_RESET_ALL.
SELECT name FROM tab_settings_flags
WHERE no_reset AND NOT no_reset_all
ORDER BY 1;
DROP TABLE tab_settings_flags;

18
src/test/regress/sql/transactions.sql
View File

@ -35,6 +35,24 @@ SELECT oid FROM pg_class WHERE relname = 'disappear';
-- should have members again
SELECT * FROM xacttest;
-- Test that transaction characteristics cannot be reset.
BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
SELECT COUNT(*) FROM xacttest;
RESET transaction_isolation; -- error
END;
BEGIN TRANSACTION READ ONLY;
SELECT COUNT(*) FROM xacttest;
RESET transaction_read_only; -- error
END;
BEGIN TRANSACTION DEFERRABLE;
SELECT COUNT(*) FROM xacttest;
RESET transaction_deferrable; -- error
END;
CREATE FUNCTION errfunc() RETURNS int LANGUAGE SQL AS 'SELECT 1'
SET transaction_read_only = on; -- error
-- Read-only tests