On Sun, Feb 3, 2013 at 07:19:14AM +0000, Amit kapila wrote:
>
> On Saturday, February 02, 2013 9:08 PM Robert Haas wrote:
> On Fri, Feb 1, 2013 at 12:04 AM, Amit Kapila <[email protected]> wrote:
> >> I think user should be aware of effect before using SET commands, as these
> >> are used at various levels (TRANSACTION, SESSION, ...).
>
> > Ideally, sure. But these kinds of mistakes are easy to make.
>
> You mean to say that after using SET Transaction, user can think below
> statements will
> use modified transaction property. But I think if user doesn't understand
> by default
> transaction will be committed after the statement execution, he might
> expect after
> few statements he can rollback.
>
> > That's why LOCK and DECLARE CURSOR already emit errors in this case - why
> > should this one be any different?
>
> IMO, I think error should be given when it is not possible to execute current
> statement.
>
> Not only LOCK,DECLARE CURSOR but SAVEPOINT and some other statements also
> give the same error,
> so if we want to throw error for such behavior, we can find all such similar
> statements
> (SET TRANSACTION, SET LOCAL, etc) and do it for all.
>
> This can be helpful to some users, but not sure if such behavior (statement
> can be executed but it will not have any sense)
> can be always detectable and maintaible.
I have created the attached patch which issues an error when SET
TRANSACTION and SET LOCAL are used outside of transactions:
test=> set transaction isolation level serializable;
ERROR: SET TRANSACTION can only be used in transaction blocks
test=> reset transaction isolation level;
ERROR: RESET TRANSACTION can only be used in transaction blocks
test=> set local effective_cache_size = '3MB';
ERROR: SET LOCAL can only be used in transaction blocks
test=> set local effective_cache_size = default;
ERROR: SET LOCAL can only be used in transaction blocks
--
Bruce Momjian <[email protected]> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
new file mode 100644
index b1023c4..0da041b
*** a/src/backend/tcop/utility.c
--- b/src/backend/tcop/utility.c
*************** standard_ProcessUtility(Node *parsetree,
*** 688,694 ****
break;
case T_VariableSetStmt:
! ExecSetVariableStmt((VariableSetStmt *) parsetree);
break;
case T_VariableShowStmt:
--- 688,694 ----
break;
case T_VariableSetStmt:
! ExecSetVariableStmt((VariableSetStmt *) parsetree, isTopLevel);
break;
case T_VariableShowStmt:
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index 7d297bc..168cd95
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** flatten_set_variable_args(const char *na
*** 6240,6246 ****
* SET command
*/
void
! ExecSetVariableStmt(VariableSetStmt *stmt)
{
GucAction action = stmt->is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET;
--- 6240,6246 ----
* SET command
*/
void
! ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
{
GucAction action = stmt->is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET;
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6248,6253 ****
--- 6248,6255 ----
{
case VAR_SET_VALUE:
case VAR_SET_CURRENT:
+ if (stmt->is_local)
+ RequireTransactionChain(isTopLevel, "SET LOCAL");
(void) set_config_option(stmt->name,
ExtractSetVariableArgs(stmt),
(superuser() ? PGC_SUSET : PGC_USERSET),
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6257,6263 ****
0);
break;
case VAR_SET_MULTI:
-
/*
* Special-case SQL syntaxes. The TRANSACTION and SESSION
* CHARACTERISTICS cases effectively set more than one variable
--- 6259,6264 ----
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6269,6274 ****
--- 6270,6277 ----
{
ListCell *head;
+ RequireTransactionChain(isTopLevel, "SET TRANSACTION");
+
foreach(head, stmt->args)
{
DefElem *item = (DefElem *) lfirst(head);
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6313,6318 ****
--- 6316,6323 ----
{
A_Const *con = (A_Const *) linitial(stmt->args);
+ RequireTransactionChain(isTopLevel, "SET TRANSACTION");
+
if (stmt->is_local)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6326,6332 ****
--- 6331,6343 ----
stmt->name);
break;
case VAR_SET_DEFAULT:
+ if (stmt->is_local)
+ RequireTransactionChain(isTopLevel, "SET LOCAL");
+ /* fall through */
case VAR_RESET:
+ if (strcmp(stmt->name, "transaction_isolation") == 0)
+ RequireTransactionChain(isTopLevel, "RESET TRANSACTION");
+
(void) set_config_option(stmt->name,
NULL,
(superuser() ? PGC_SUSET : PGC_USERSET),
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
new file mode 100644
index 99211c1..89ee40c
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
*************** extern void SetPGVariable(const char *na
*** 334,340 ****
extern void GetPGVariable(const char *name, DestReceiver *dest);
extern TupleDesc GetPGVariableResultDesc(const char *name);
! extern void ExecSetVariableStmt(VariableSetStmt *stmt);
extern char *ExtractSetVariableArgs(VariableSetStmt *stmt);
extern void ProcessGUCArray(ArrayType *array,
--- 334,340 ----
extern void GetPGVariable(const char *name, DestReceiver *dest);
extern TupleDesc GetPGVariableResultDesc(const char *name);
! extern void ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel);
extern char *ExtractSetVariableArgs(VariableSetStmt *stmt);
extern void ProcessGUCArray(ArrayType *array,
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
new file mode 100644
index 7b5a624..203fa6e
*** a/src/test/regress/expected/guc.out
--- b/src/test/regress/expected/guc.out
*************** SELECT '2006-08-13 12:34:56'::timestampt
*** 29,34 ****
--- 29,35 ----
-- SET LOCAL has no effect outside of a transaction
SET LOCAL vacuum_cost_delay TO 50;
+ ERROR: SET LOCAL can only be used in transaction blocks
SHOW vacuum_cost_delay;
vacuum_cost_delay
-------------------
*************** SHOW vacuum_cost_delay;
*** 36,41 ****
--- 37,43 ----
(1 row)
SET LOCAL datestyle = 'SQL';
+ ERROR: SET LOCAL can only be used in transaction blocks
SHOW datestyle;
DateStyle
-----------
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
