Re: [v9.2] Add GUC sepgsql.client_label
Started by Kevin Grittneralmost 14 years ago1 messages
"Kevin Grittner" wrote:
Tom Lane wrote:
I agree it's a bug that you can do what Kevin's example shows.
I'll look at it and see if I can pull together a patch.
Attached.
Basically, if a GUC has a check function, this patch causes it to be
run on a RESET just like it is on a SET, to make sure that the
resulting value is valid to set within the context. Some messages
needed adjustment. While I was there, I made cod a little more
consistent among related GUCs.
I also added a little to the regression tests to cover this.
-Kevin
Attachments:
check-reset-v1.patchapplication/octet-stream; name=check-reset-v1.patchDownload
*** 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
***************
*** 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
{
--- 5287,5296 ----
***************
*** 5306,5311 **** set_config_option(const char *name, const char *value,
--- 5300,5309 ----
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
{
--- 5389,5398 ----
***************
*** 5410,5415 **** set_config_option(const char *name, const char *value,
--- 5402,5411 ----
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
{
--- 5488,5497 ----
***************
*** 5511,5516 **** set_config_option(const char *name, const char *value,
--- 5501,5510 ----
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)
{
--- 5585,5590 ----
***************
*** 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 */
--- 5597,5626 ----
}
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
{
--- 5712,5721 ----
***************
*** 5740,5745 **** set_config_option(const char *name, const char *value,
--- 5725,5734 ----
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,82 ----
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;
+ RESET default_transaction_read_only;
+ RESET default_transaction_isolation;
BEGIN;
SET TRANSACTION READ ONLY; -- ok
SET TRANSACTION READ WRITE; -- ok
*** a/src/test/regress/sql/transactions.sql
--- b/src/test/regress/sql/transactions.sql
***************
*** 45,50 **** SELECT * FROM writetest; -- ok
--- 45,68 ----
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;
+
+ RESET default_transaction_read_only;
+ RESET default_transaction_isolation;
+
BEGIN;
SET TRANSACTION READ ONLY; -- ok
SET TRANSACTION READ WRITE; -- ok