On Sun, Sep 29, 2013 at 11:40:51AM +0530, Amit Kapila wrote:
> >> Shouldn't we do it for Set Constraints as well?
> >
> > Oh, very good point. I missed that one. Updated patch attached.
I am glad you are seeing things I am not. :-)
> 1. The function set_config also needs similar functionality, else
> there will be inconsistency, the SQL statement will give error but
> equivalent function set_config() will succeed.
>
> SQL Command
> postgres=# set local search_path='public';
> ERROR: SET LOCAL can only be used in transaction blocks
>
> Function
> postgres=# select set_config('search_path', 'public', true);
> set_config
> ------------
> public
> (1 row)
I looked at this but could not see how to easily pass the value of
'isTopLevel' down to the SELECT. All the other checks have isTopLevel
passed down from the utility case statement.
> 2. I think we should update the documentation as well.
>
> For example:
> The documentation of LOCK TABLE
> (http://www.postgresql.org/docs/devel/static/sql-lock.html) clearly
> indicates that it will give error if used outside transaction block.
>
> "LOCK TABLE is useless outside a transaction block: the lock would
> remain held only to the completion of the statement. Therefore
> PostgreSQL reports an error if LOCK is used outside a transaction
> block. Use BEGINand COMMIT (or ROLLBACK) to define a transaction
> block."
>
>
> The documentation of SET TRANSACTION
> (http://www.postgresql.org/docs/devel/static/sql-set-transaction.html)
> has below line which doesn't indicate that it will give error if used
> outside transaction block.
>
> "If SET TRANSACTION is executed without a prior START TRANSACTION or
> BEGIN, it will appear to have no effect, since the transaction will
> immediately end."
Yes, you are right. All cases I changed had mentions of the command
having no effect; I have updated it to mention an error is generated.
> 3.
>
> void
> ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
> {
> ..
> ..
> else if (strcmp(stmt->name, "TRANSACTION SNAPSHOT") == 0)
> {
> A_Const *con = (A_Const *) linitial(stmt->args);
>
> RequireTransactionChain(isTopLevel, "SET TRANSACTION");
>
> if (stmt->is_local)
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("SET LOCAL TRANSACTION SNAPSHOT is not implemented")));
> ..
> }
> ..
> ..
> }
>
> Wouldn't it be better if call to RequireTransactionChain() is done
> after if (stmt->is_local)?
Yes, good point. Done.
Thanks much for your review. Updated patch attached.
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/doc/src/sgml/ref/set.sgml b/doc/src/sgml/ref/set.sgml
new file mode 100644
index 21745db..d108dd4
*** a/doc/src/sgml/ref/set.sgml
--- b/doc/src/sgml/ref/set.sgml
*************** SET [ SESSION | LOCAL ] TIME ZONE { <rep
*** 110,119 ****
<para>
Specifies that the command takes effect for only the current
transaction. After <command>COMMIT</> or <command>ROLLBACK</>,
! the session-level setting takes effect again. Note that
! <command>SET LOCAL</> will appear to have no effect if it is
! executed outside a <command>BEGIN</> block, since the
! transaction will end immediately.
</para>
</listitem>
</varlistentry>
--- 110,118 ----
<para>
Specifies that the command takes effect for only the current
transaction. After <command>COMMIT</> or <command>ROLLBACK</>,
! the session-level setting takes effect again.
! <productname>PostgreSQL</productname> reports an error if
! <command>SET LOCAL</> is used outside a transaction block.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/set_constraints.sgml b/doc/src/sgml/ref/set_constraints.sgml
new file mode 100644
index 8098b7b..895a5fd
*** a/doc/src/sgml/ref/set_constraints.sgml
--- b/doc/src/sgml/ref/set_constraints.sgml
*************** SET CONSTRAINTS { ALL | <replaceable cla
*** 102,108 ****
current transaction. Thus, if you execute this command outside of a
transaction block
(<command>BEGIN</command>/<command>COMMIT</command> pair), it will
! not appear to have any effect.
</para>
</refsect1>
--- 102,108 ----
current transaction. Thus, if you execute this command outside of a
transaction block
(<command>BEGIN</command>/<command>COMMIT</command> pair), it will
! generate an error.
</para>
</refsect1>
diff --git a/doc/src/sgml/ref/set_transaction.sgml b/doc/src/sgml/ref/set_transaction.sgml
new file mode 100644
index f060729..391464a
*** a/doc/src/sgml/ref/set_transaction.sgml
--- b/doc/src/sgml/ref/set_transaction.sgml
*************** SET SESSION CHARACTERISTICS AS TRANSACTI
*** 184,192 ****
<para>
If <command>SET TRANSACTION</command> is executed without a prior
! <command>START TRANSACTION</command> or <command>BEGIN</command>,
! it will appear to have no effect, since the transaction will immediately
! end.
</para>
<para>
--- 184,191 ----
<para>
If <command>SET TRANSACTION</command> is executed without a prior
! <command>START TRANSACTION</command> or <command>BEGIN</command>,
! it will generate an error.
</para>
<para>
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
new file mode 100644
index b1023c4..3ffdfe0
*** 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:
*************** standard_ProcessUtility(Node *parsetree,
*** 754,759 ****
--- 754,760 ----
break;
case T_ConstraintsSetStmt:
+ RequireTransactionChain(isTopLevel, "SET CONSTRAINTS");
AfterTriggerSetState((ConstraintsSetStmt *) parsetree);
break;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index 3107f9c..d9a06b4
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** flatten_set_variable_args(const char *na
*** 6252,6258 ****
* SET command
*/
void
! ExecSetVariableStmt(VariableSetStmt *stmt)
{
GucAction action = stmt->is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET;
--- 6252,6258 ----
* SET command
*/
void
! ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
{
GucAction action = stmt->is_local ? GUC_ACTION_LOCAL : GUC_ACTION_SET;
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6260,6265 ****
--- 6260,6267 ----
{
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
*** 6269,6275 ****
0);
break;
case VAR_SET_MULTI:
-
/*
* Special-case SQL syntaxes. The TRANSACTION and SESSION
* CHARACTERISTICS cases effectively set more than one variable
--- 6271,6276 ----
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6281,6286 ****
--- 6282,6289 ----
{
ListCell *head;
+ RequireTransactionChain(isTopLevel, "SET TRANSACTION");
+
foreach(head, stmt->args)
{
DefElem *item = (DefElem *) lfirst(head);
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6329,6334 ****
--- 6332,6339 ----
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("SET LOCAL TRANSACTION SNAPSHOT is not implemented")));
+
+ RequireTransactionChain(isTopLevel, "SET TRANSACTION");
Assert(IsA(con, A_Const));
Assert(nodeTag(&con->val) == T_String);
ImportSnapshot(strVal(&con->val));
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6338,6344 ****
--- 6343,6355 ----
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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
