*** a/src/backend/commands/variable.c --- b/src/backend/commands/variable.c *************** *** 600,616 **** check_XactIsoLevel(char **newval, void **extra, GucSource source) if (newXactIsoLevel != XactIsoLevel) { ! if (FirstSnapshotSet) { GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION); ! GUC_check_errmsg("SET TRANSACTION ISOLATION LEVEL must be called before any query"); return false; } ! /* We ignore a subtransaction setting it to the existing value. */ ! if (IsSubTransaction()) { GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION); ! GUC_check_errmsg("SET TRANSACTION ISOLATION LEVEL must not be called in a subtransaction"); return false; } /* Can't go to serializable mode while recovery is still active */ --- 600,616 ---- if (newXactIsoLevel != XactIsoLevel) { ! /* We ignore a subtransaction setting it to the existing value. */ ! if (IsSubTransaction()) { GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION); ! GUC_check_errmsg("cannot set transaction isolation level in a subtransaction"); return false; } ! if (FirstSnapshotSet) { GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION); ! GUC_check_errmsg("transaction isolation level must be set before any query"); return false; } /* Can't go to serializable mode while recovery is still active */ *************** *** 665,677 **** check_transaction_deferrable(bool *newval, void **extra, GucSource source) if (IsSubTransaction()) { GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION); ! GUC_check_errmsg("SET TRANSACTION [NOT] DEFERRABLE cannot be called within a subtransaction"); return false; } if (FirstSnapshotSet) { GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION); ! GUC_check_errmsg("SET TRANSACTION [NOT] DEFERRABLE must be called before any query"); return false; } --- 665,677 ---- if (IsSubTransaction()) { GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION); ! GUC_check_errmsg("cannot set transaction deferrable state within a subtransaction"); return false; } if (FirstSnapshotSet) { GUC_check_errcode(ERRCODE_ACTIVE_SQL_TRANSACTION); ! GUC_check_errmsg("transaction deferrable state must be set before any query"); return false; } *** a/src/backend/utils/misc/guc.c --- b/src/backend/utils/misc/guc.c *************** *** 5042,5047 **** config_enum_get_options(struct config_enum * record, const char *prefix, --- 5042,5051 ---- * * If value is NULL, set the option to its default value (normally the * reset_val, but if source == PGC_S_DEFAULT we instead use the boot_val). + * When we reset a value we can't assume that the old value is valid, but must + * call the check hook if present. This is because the validity of a change + * might depend on context. For example, transaction isolation may not be + * changed after the transaction has run a query, even by a RESET command. * * action indicates whether to set the value globally in the session, locally * to the current top transaction, or just for the duration of a function call. *************** *** 5287,5302 **** set_config_option(const char *name, const char *value, name))); return 0; } - if (!call_bool_check_hook(conf, &newval, &newextra, - source, elevel)) - return 0; } else if (source == PGC_S_DEFAULT) { newval = conf->boot_val; - if (!call_bool_check_hook(conf, &newval, &newextra, - source, elevel)) - return 0; } else { --- 5291,5300 ---- *************** *** 5306,5311 **** set_config_option(const char *name, const char *value, --- 5304,5313 ---- context = conf->gen.reset_scontext; } + if (!call_bool_check_hook(conf, &newval, &newextra, + source, elevel)) + return 0; + if (prohibitValueChange) { if (*conf->variable != newval) *************** *** 5391,5406 **** set_config_option(const char *name, const char *value, newval, name, conf->min, conf->max))); return 0; } - if (!call_int_check_hook(conf, &newval, &newextra, - source, elevel)) - return 0; } else if (source == PGC_S_DEFAULT) { newval = conf->boot_val; - if (!call_int_check_hook(conf, &newval, &newextra, - source, elevel)) - return 0; } else { --- 5393,5402 ---- *************** *** 5410,5415 **** set_config_option(const char *name, const char *value, --- 5406,5415 ---- context = conf->gen.reset_scontext; } + if (!call_int_check_hook(conf, &newval, &newextra, + source, elevel)) + return 0; + if (prohibitValueChange) { if (*conf->variable != newval) *************** *** 5492,5507 **** set_config_option(const char *name, const char *value, newval, name, conf->min, conf->max))); return 0; } - if (!call_real_check_hook(conf, &newval, &newextra, - source, elevel)) - return 0; } else if (source == PGC_S_DEFAULT) { newval = conf->boot_val; - if (!call_real_check_hook(conf, &newval, &newextra, - source, elevel)) - return 0; } else { --- 5492,5501 ---- *************** *** 5511,5516 **** set_config_option(const char *name, const char *value, --- 5505,5514 ---- context = conf->gen.reset_scontext; } + if (!call_real_check_hook(conf, &newval, &newextra, + source, elevel)) + return 0; + if (prohibitValueChange) { if (*conf->variable != newval) *************** *** 5591,5603 **** set_config_option(const char *name, const char *value, */ if (conf->gen.flags & GUC_IS_NAME) truncate_identifier(newval, strlen(newval), true); - - if (!call_string_check_hook(conf, &newval, &newextra, - source, elevel)) - { - free(newval); - return 0; - } } else if (source == PGC_S_DEFAULT) { --- 5589,5594 ---- *************** *** 5610,5635 **** set_config_option(const char *name, const char *value, } else newval = NULL; - - if (!call_string_check_hook(conf, &newval, &newextra, - source, elevel)) - { - free(newval); - return 0; - } } else { ! /* ! * strdup not needed, since reset_val is already under ! * guc.c's control ! */ ! newval = conf->reset_val; newextra = conf->reset_extra; source = conf->gen.reset_source; context = conf->gen.reset_scontext; } if (prohibitValueChange) { /* newval shouldn't be NULL, so we're a bit sloppy here */ --- 5601,5630 ---- } else newval = NULL; } else { ! /* non-NULL reset_val must always get strdup'd */ ! if (conf->reset_val != NULL) ! { ! newval = guc_strdup(elevel, conf->reset_val); ! if (newval == NULL) ! return 0; ! } ! else ! newval = NULL; newextra = conf->reset_extra; source = conf->gen.reset_source; context = conf->gen.reset_scontext; } + if (!call_string_check_hook(conf, &newval, &newextra, + source, elevel)) + { + free(newval); + return 0; + } + if (prohibitValueChange) { /* newval shouldn't be NULL, so we're a bit sloppy here */ *************** *** 5721,5736 **** set_config_option(const char *name, const char *value, pfree(hintmsg); return 0; } - if (!call_enum_check_hook(conf, &newval, &newextra, - source, elevel)) - return 0; } else if (source == PGC_S_DEFAULT) { newval = conf->boot_val; - if (!call_enum_check_hook(conf, &newval, &newextra, - source, elevel)) - return 0; } else { --- 5716,5725 ---- *************** *** 5740,5745 **** set_config_option(const char *name, const char *value, --- 5729,5738 ---- context = conf->gen.reset_scontext; } + if (!call_enum_check_hook(conf, &newval, &newextra, + source, elevel)) + return 0; + if (prohibitValueChange) { if (*conf->variable != newval) *** a/src/test/regress/expected/transactions.out --- b/src/test/regress/expected/transactions.out *************** *** 53,58 **** SELECT * FROM writetest; -- ok --- 53,90 ---- SET TRANSACTION READ WRITE; --fail ERROR: transaction read-write mode must be set before any query COMMIT; + SET default_transaction_read_only = FALSE; + SET default_transaction_isolation = 'read committed'; + BEGIN; + SET TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ ONLY; -- ok + SELECT * FROM writetest; -- ok + a + --- + (0 rows) + + RESET transaction_read_only; --fail + ERROR: transaction read-write mode must be set before any query + COMMIT; + BEGIN; + SET TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ ONLY; -- ok + SELECT * FROM writetest; -- ok + a + --- + (0 rows) + + RESET transaction_isolation; --fail + ERROR: transaction isolation level must be set before any query + COMMIT; + BEGIN; + SET TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ ONLY; -- ok + SELECT * FROM writetest; -- ok + a + --- + (0 rows) + + ROLLBACK; -- ok + RESET default_transaction_read_only; + RESET default_transaction_isolation; BEGIN; SET TRANSACTION READ ONLY; -- ok SET TRANSACTION READ WRITE; -- ok *************** *** 391,396 **** SELECT 1; -- this should work --- 423,480 ---- 1 (1 row) + -- test rollbacks and resets of GUC in transactions + CREATE SCHEMA albion; + SET search_path = "$user",public,albion; + DROP SCHEMA albion; + SHOW search_path; + search_path + ------------------------- + "$user", public, albion + (1 row) + + BEGIN; + CREATE SCHEMA camelot; + SET search_path = "$user",public,camelot; + DROP SCHEMA camelot; + SAVEPOINT bedivere; + CREATE SCHEMA avalon; + SET search_path = "$user",public,avalon; + DROP SCHEMA avalon; + SHOW search_path; + search_path + ------------------------- + "$user", public, avalon + (1 row) + + ROLLBACK TO SAVEPOINT bedivere; + SHOW search_path; + search_path + -------------------------- + "$user", public, camelot + (1 row) + + RESET search_path; + SHOW search_path; + search_path + ---------------- + "$user",public + (1 row) + + ROLLBACK; + SHOW search_path; + search_path + ------------------------- + "$user", public, albion + (1 row) + + RESET search_path; + SHOW search_path; + search_path + ---------------- + "$user",public + (1 row) + -- check non-transactional behavior of cursors BEGIN; DECLARE c CURSOR FOR SELECT unique2 FROM tenk1 ORDER BY unique2; *** a/src/test/regress/sql/transactions.sql --- b/src/test/regress/sql/transactions.sql *************** *** 45,50 **** SELECT * FROM writetest; -- ok --- 45,73 ---- SET TRANSACTION READ WRITE; --fail COMMIT; + SET default_transaction_read_only = FALSE; + SET default_transaction_isolation = 'read committed'; + + BEGIN; + SET TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ ONLY; -- ok + SELECT * FROM writetest; -- ok + RESET transaction_read_only; --fail + COMMIT; + + BEGIN; + SET TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ ONLY; -- ok + SELECT * FROM writetest; -- ok + RESET transaction_isolation; --fail + COMMIT; + + BEGIN; + SET TRANSACTION ISOLATION LEVEL SERIALIZABLE, READ ONLY; -- ok + SELECT * FROM writetest; -- ok + ROLLBACK; -- ok + + RESET default_transaction_read_only; + RESET default_transaction_isolation; + BEGIN; SET TRANSACTION READ ONLY; -- ok SET TRANSACTION READ WRITE; -- ok *************** *** 252,257 **** BEGIN; --- 275,303 ---- COMMIT; SELECT 1; -- this should work + -- test rollbacks and resets of GUC in transactions + CREATE SCHEMA albion; + SET search_path = "$user",public,albion; + DROP SCHEMA albion; + SHOW search_path; + BEGIN; + CREATE SCHEMA camelot; + SET search_path = "$user",public,camelot; + DROP SCHEMA camelot; + SAVEPOINT bedivere; + CREATE SCHEMA avalon; + SET search_path = "$user",public,avalon; + DROP SCHEMA avalon; + SHOW search_path; + ROLLBACK TO SAVEPOINT bedivere; + SHOW search_path; + RESET search_path; + SHOW search_path; + ROLLBACK; + SHOW search_path; + RESET search_path; + SHOW search_path; + -- check non-transactional behavior of cursors BEGIN; DECLARE c CURSOR FOR SELECT unique2 FROM tenk1 ORDER BY unique2;