On Fri, Nov 22, 2013 at 10:24:35AM -0300, Alvaro Herrera wrote:
> Bruce Momjian escribió:
>
> > OK, here is a patch which changes ABORT from NOTICE to WARNING, and SET
> > from ERROR (which is new in 9.4) to WARNING.
>
> I don't like that this patch changes RequireTransactionChain() from
> actually requiring one, to a function that maybe requires a transaction
> chain, and maybe it only complains about there not being one. I mean,
> it's like you had named the new throwError boolean as "notReally" or
> something like that. Also, the new comment paragraph is bad because it
> explains who must pass true/false, instead of what's the effect of each
> value (and let the callers choose which value to pass).
>
> I would create a separate function to implement this, maybe
> WarnUnlessInTransactionBlock() or something like that. That would make
> the patch a good deal smaller (because not changing existing callers).
Good points. I have modified the attached patch to do as you suggested.
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/doc/src/sgml/ref/abort.sgml b/doc/src/sgml/ref/abort.sgml
new file mode 100644
index 246e8f8..f3a2fa8
*** a/doc/src/sgml/ref/abort.sgml
--- b/doc/src/sgml/ref/abort.sgml
*************** ABORT [ WORK | TRANSACTION ]
*** 63,70 ****
</para>
<para>
! Issuing <command>ABORT</> when not inside a transaction does
! no harm, but it will provoke a warning message.
</para>
</refsect1>
--- 63,69 ----
</para>
<para>
! Issuing <command>ABORT</> outside of a transaction block has no effect.
</para>
</refsect1>
diff --git a/doc/src/sgml/ref/rollback.sgml b/doc/src/sgml/ref/rollback.sgml
new file mode 100644
index b265545..4f79621
*** a/doc/src/sgml/ref/rollback.sgml
--- b/doc/src/sgml/ref/rollback.sgml
*************** ROLLBACK [ WORK | TRANSACTION ]
*** 59,66 ****
</para>
<para>
! Issuing <command>ROLLBACK</> when not inside a transaction does
! no harm, but it will provoke a warning message.
</para>
</refsect1>
--- 59,66 ----
</para>
<para>
! Issuing <command>ROLLBACK</> outside of a transaction
! block has no effect.
</para>
</refsect1>
diff --git a/doc/src/sgml/ref/set.sgml b/doc/src/sgml/ref/set.sgml
new file mode 100644
index 6290c9d..5a84f69
*** a/doc/src/sgml/ref/set.sgml
--- b/doc/src/sgml/ref/set.sgml
*************** SET [ SESSION | LOCAL ] TIME ZONE { <rep
*** 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>
--- 110,117 ----
<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. This has no effect
! outside of 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 895a5fd..a33190c
*** a/doc/src/sgml/ref/set_constraints.sgml
--- b/doc/src/sgml/ref/set_constraints.sgml
*************** SET CONSTRAINTS { ALL | <replaceable cla
*** 99,108 ****
<para>
This command only alters the behavior of constraints within the
! 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>
--- 99,105 ----
<para>
This command only alters the behavior of constraints within the
! current transaction. This has no effect outside of a transaction block.
</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 391464a..e90ff4a
*** a/doc/src/sgml/ref/set_transaction.sgml
--- b/doc/src/sgml/ref/set_transaction.sgml
*************** SET SESSION CHARACTERISTICS AS TRANSACTI
*** 185,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>
--- 185,191 ----
<para>
If <command>SET TRANSACTION</command> is executed without a prior
<command>START TRANSACTION</command> or <command>BEGIN</command>,
! it will have no effect.
</para>
<para>
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
new file mode 100644
index 0591f3f..bab048d
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** static void CallSubXactCallbacks(SubXact
*** 265,270 ****
--- 265,272 ----
SubTransactionId mySubid,
SubTransactionId parentSubid);
static void CleanupTransaction(void);
+ static void CheckTransactionChain(bool isTopLevel, bool throwError,
+ const char *stmtType);
static void CommitTransaction(void);
static TransactionId RecordTransactionAbort(bool isSubXact);
static void StartTransaction(void);
*************** PreventTransactionChain(bool isTopLevel,
*** 2949,2954 ****
--- 2951,2976 ----
}
/*
+ * These two functions allow for warnings or errors if a command is
+ * executed outside of a transaction block.
+ *
+ * While top-level transaction control commands (BEGIN/COMMIT/ABORT) and
+ * SET that have no effect issue warnings, all other no-effect commands
+ * generate errors.
+ */
+ void
+ WarnNoTransactionChain(bool isTopLevel, const char *stmtType)
+ {
+ CheckTransactionChain(isTopLevel, false, stmtType);
+ }
+
+ void
+ RequireTransactionChain(bool isTopLevel, const char *stmtType)
+ {
+ CheckTransactionChain(isTopLevel, true, stmtType);
+ }
+
+ /*
* RequireTransactionChain
*
* This routine is to be called by statements that must run inside
*************** PreventTransactionChain(bool isTopLevel,
*** 2957,2972 ****
* is presumably an error). DECLARE CURSOR is an example.
*
* If we appear to be running inside a user-defined function, we do not
! * issue an error, since the function could issue more commands that make
* use of the current statement's results. Likewise subtransactions.
* Thus this is an inverse for PreventTransactionChain.
*
* isTopLevel: passed down from ProcessUtility to determine whether we are
* inside a function.
! * stmtType: statement type name, for error messages.
*/
! void
! RequireTransactionChain(bool isTopLevel, const char *stmtType)
{
/*
* xact block already started?
--- 2979,2994 ----
* is presumably an error). DECLARE CURSOR is an example.
*
* If we appear to be running inside a user-defined function, we do not
! * issue anything, since the function could issue more commands that make
* use of the current statement's results. Likewise subtransactions.
* Thus this is an inverse for PreventTransactionChain.
*
* isTopLevel: passed down from ProcessUtility to determine whether we are
* inside a function.
! * stmtType: statement type name, for warning or error messages.
*/
! static void
! CheckTransactionChain(bool isTopLevel, bool throwError, const char *stmtType)
{
/*
* xact block already started?
*************** RequireTransactionChain(bool isTopLevel,
*** 2986,2996 ****
if (!isTopLevel)
return;
! ereport(ERROR,
(errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
/* translator: %s represents an SQL statement name */
errmsg("%s can only be used in transaction blocks",
stmtType)));
}
/*
--- 3008,3019 ----
if (!isTopLevel)
return;
! ereport(throwError ? ERROR : WARNING,
(errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
/* translator: %s represents an SQL statement name */
errmsg("%s can only be used in transaction blocks",
stmtType)));
+ return;
}
/*
*************** UserAbortTransactionBlock(void)
*** 3425,3436 ****
/*
* The user issued ABORT when not inside a transaction. Issue a
! * NOTICE and go to abort state. The upcoming call to
* CommitTransactionCommand() will then put us back into the
* default state.
*/
case TBLOCK_STARTED:
! ereport(NOTICE,
(errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
errmsg("there is no transaction in progress")));
s->blockState = TBLOCK_ABORT_PENDING;
--- 3448,3459 ----
/*
* The user issued ABORT when not inside a transaction. Issue a
! * WARNING and go to abort state. The upcoming call to
* CommitTransactionCommand() will then put us back into the
* default state.
*/
case TBLOCK_STARTED:
! ereport(WARNING,
(errcode(ERRCODE_NO_ACTIVE_SQL_TRANSACTION),
errmsg("there is no transaction in progress")));
s->blockState = TBLOCK_ABORT_PENDING;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
new file mode 100644
index 6a7bf0d..5895102
*** a/src/backend/tcop/utility.c
--- b/src/backend/tcop/utility.c
*************** standard_ProcessUtility(Node *parsetree,
*** 754,760 ****
break;
case T_ConstraintsSetStmt:
! RequireTransactionChain(isTopLevel, "SET CONSTRAINTS");
AfterTriggerSetState((ConstraintsSetStmt *) parsetree);
break;
--- 754,760 ----
break;
case T_ConstraintsSetStmt:
! WarnNoTransactionChain(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 54d8078..cbf3186
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6274,6280 ****
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),
--- 6274,6280 ----
case VAR_SET_VALUE:
case VAR_SET_CURRENT:
if (stmt->is_local)
! WarnNoTransactionChain(isTopLevel, "SET LOCAL");
(void) set_config_option(stmt->name,
ExtractSetVariableArgs(stmt),
(superuser() ? PGC_SUSET : PGC_USERSET),
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6295,6301 ****
{
ListCell *head;
! RequireTransactionChain(isTopLevel, "SET TRANSACTION");
foreach(head, stmt->args)
{
--- 6295,6301 ----
{
ListCell *head;
! WarnNoTransactionChain(isTopLevel, "SET TRANSACTION");
foreach(head, stmt->args)
{
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6346,6352 ****
(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));
--- 6346,6352 ----
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("SET LOCAL TRANSACTION SNAPSHOT is not implemented")));
! WarnNoTransactionChain(isTopLevel, "SET TRANSACTION");
Assert(IsA(con, A_Const));
Assert(nodeTag(&con->val) == T_String);
ImportSnapshot(strVal(&con->val));
*************** ExecSetVariableStmt(VariableSetStmt *stm
*** 6357,6367 ****
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,
--- 6357,6367 ----
break;
case VAR_SET_DEFAULT:
if (stmt->is_local)
! WarnNoTransactionChain(isTopLevel, "SET LOCAL");
/* fall through */
case VAR_RESET:
if (strcmp(stmt->name, "transaction_isolation") == 0)
! WarnNoTransactionChain(isTopLevel, "RESET TRANSACTION");
(void) set_config_option(stmt->name,
NULL,
diff --git a/src/include/access/xact.h b/src/include/access/xact.h
new file mode 100644
index 835f6ac..1d3e7d8
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** extern char TransactionBlockStatusCode(v
*** 245,250 ****
--- 245,251 ----
extern void AbortOutOfAnyTransaction(void);
extern void PreventTransactionChain(bool isTopLevel, const char *stmtType);
extern void RequireTransactionChain(bool isTopLevel, const char *stmtType);
+ extern void WarnNoTransactionChain(bool isTopLevel, const char *stmtType);
extern bool IsInTransactionChain(bool isTopLevel);
extern void RegisterXactCallback(XactCallback callback, void *arg);
extern void UnregisterXactCallback(XactCallback callback, void *arg);
diff --git a/src/test/regress/expected/errors.out b/src/test/regress/expected/errors.out
new file mode 100644
index fa0bd82..4061512
*** a/src/test/regress/expected/errors.out
--- b/src/test/regress/expected/errors.out
*************** ERROR: column name "oid" conflicts with
*** 114,120 ****
-- TRANSACTION STUFF
-- not in a xact
abort;
! NOTICE: there is no transaction in progress
-- not in a xact
end;
WARNING: there is no transaction in progress
--- 114,120 ----
-- TRANSACTION STUFF
-- not in a xact
abort;
! WARNING: there is no transaction in progress
-- not in a xact
end;
WARNING: there is no transaction in progress
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
new file mode 100644
index 203fa6e..4f0065c
*** a/src/test/regress/expected/guc.out
--- b/src/test/regress/expected/guc.out
*************** SELECT '2006-08-13 12:34:56'::timestampt
*** 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
-------------------
--- 29,35 ----
-- SET LOCAL has no effect outside of a transaction
SET LOCAL vacuum_cost_delay TO 50;
! WARNING: SET LOCAL can only be used in transaction blocks
SHOW vacuum_cost_delay;
vacuum_cost_delay
-------------------
*************** SHOW vacuum_cost_delay;
*** 37,43 ****
(1 row)
SET LOCAL datestyle = 'SQL';
! ERROR: SET LOCAL can only be used in transaction blocks
SHOW datestyle;
DateStyle
-----------
--- 37,43 ----
(1 row)
SET LOCAL datestyle = 'SQL';
! WARNING: 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
