On Wed, Nov 20, 2013 at 10:16:00AM -0500, Bruce Momjian wrote:
> > > The attached patch changes ABORT from NOTICE to WARNING, and documents
> > > that all other are errors. This "top-level" logic idea came from Robert
> > > Haas, and it has some level of consistency.
> >
> > This patch utterly fails to address my complaint.
> >
> > More to the point, I think it's a waste of time to make these sorts of
> > adjustments. The only thanks you're likely to get for it is complaints
> > about cross-version behavioral changes. Also, you're totally ignoring
> > the thought that these different message levels might've been selected
> > intentionally, back when the code was written. Since there have been
> > no field complaints about the inconsistency, what's your hurry to
> > change it? See Emerson.
>
> My problem was that they issued _no_ message at all. I am fine with
> them issuing a warning if that's what people prefer. As they are all
> SET commands, they will be consistent.
OK, here is a patch which changes ABORT from NOTICE to WARNING, and SET
from ERROR (which is new in 9.4) to WARNING.
--
Bruce Momjian <[email protected]> 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..2cdcc98
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** PreventTransactionChain(bool isTopLevel,
*** 2961,2972 ****
* 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?
--- 2961,2976 ----
* use of the current statement's results. Likewise subtransactions.
* Thus this is an inverse for PreventTransactionChain.
*
+ * While top-level transaction control commands (BEGIN/COMMIT/ABORT) and
+ * SET that have no effect issue warnings, all other no-effect commands
+ * generate errors.
+ *
* isTopLevel: passed down from ProcessUtility to determine whether we are
* inside a function.
* stmtType: statement type name, for error messages.
*/
void
! RequireTransactionChain(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)));
}
/*
--- 2990,3001 ----
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;
--- 3430,3441 ----
/*
* 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/commands/portalcmds.c b/src/backend/commands/portalcmds.c
new file mode 100644
index 5c3f42c..c3c3e20
*** a/src/backend/commands/portalcmds.c
--- b/src/backend/commands/portalcmds.c
*************** PerformCursorOpen(PlannedStmt *stmt, Par
*** 66,72 ****
* user-visible effect).
*/
if (!(cstmt->options & CURSOR_OPT_HOLD))
! RequireTransactionChain(isTopLevel, "DECLARE CURSOR");
/*
* Create a portal and copy the plan and queryString into its memory.
--- 66,72 ----
* user-visible effect).
*/
if (!(cstmt->options & CURSOR_OPT_HOLD))
! RequireTransactionChain(isTopLevel, true, "DECLARE CURSOR");
/*
* Create a portal and copy the plan and queryString into its memory.
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
new file mode 100644
index 6a7bf0d..6d4b18b
*** a/src/backend/tcop/utility.c
--- b/src/backend/tcop/utility.c
*************** standard_ProcessUtility(Node *parsetree,
*** 461,467 ****
ListCell *cell;
char *name = NULL;
! RequireTransactionChain(isTopLevel, "SAVEPOINT");
foreach(cell, stmt->options)
{
--- 461,467 ----
ListCell *cell;
char *name = NULL;
! RequireTransactionChain(isTopLevel, true, "SAVEPOINT");
foreach(cell, stmt->options)
{
*************** standard_ProcessUtility(Node *parsetree,
*** 478,489 ****
break;
case TRANS_STMT_RELEASE:
! RequireTransactionChain(isTopLevel, "RELEASE SAVEPOINT");
ReleaseSavepoint(stmt->options);
break;
case TRANS_STMT_ROLLBACK_TO:
! RequireTransactionChain(isTopLevel, "ROLLBACK TO SAVEPOINT");
RollbackToSavepoint(stmt->options);
/*
--- 478,489 ----
break;
case TRANS_STMT_RELEASE:
! RequireTransactionChain(isTopLevel, true, "RELEASE SAVEPOINT");
ReleaseSavepoint(stmt->options);
break;
case TRANS_STMT_ROLLBACK_TO:
! RequireTransactionChain(isTopLevel, true, "ROLLBACK TO SAVEPOINT");
RollbackToSavepoint(stmt->options);
/*
*************** standard_ProcessUtility(Node *parsetree,
*** 749,760 ****
* Since the lock would just get dropped immediately, LOCK TABLE
* outside a transaction block is presumed to be user error.
*/
! RequireTransactionChain(isTopLevel, "LOCK TABLE");
LockTableCommand((LockStmt *) parsetree);
break;
case T_ConstraintsSetStmt:
! RequireTransactionChain(isTopLevel, "SET CONSTRAINTS");
AfterTriggerSetState((ConstraintsSetStmt *) parsetree);
break;
--- 749,760 ----
* Since the lock would just get dropped immediately, LOCK TABLE
* outside a transaction block is presumed to be user error.
*/
! RequireTransactionChain(isTopLevel, true, "LOCK TABLE");
LockTableCommand((LockStmt *) parsetree);
break;
case T_ConstraintsSetStmt:
! RequireTransactionChain(isTopLevel, false, "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..6dffa12
*** 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)
! RequireTransactionChain(isTopLevel, false, "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;
! RequireTransactionChain(isTopLevel, false, "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")));
! RequireTransactionChain(isTopLevel, false, "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)
! RequireTransactionChain(isTopLevel, false, "SET LOCAL");
/* fall through */
case VAR_RESET:
if (strcmp(stmt->name, "transaction_isolation") == 0)
! RequireTransactionChain(isTopLevel, false, "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..1265bea
*** a/src/include/access/xact.h
--- b/src/include/access/xact.h
*************** extern bool IsTransactionOrTransactionBl
*** 244,250 ****
extern char TransactionBlockStatusCode(void);
extern void AbortOutOfAnyTransaction(void);
extern void PreventTransactionChain(bool isTopLevel, const char *stmtType);
! extern void RequireTransactionChain(bool isTopLevel, const char *stmtType);
extern bool IsInTransactionChain(bool isTopLevel);
extern void RegisterXactCallback(XactCallback callback, void *arg);
extern void UnregisterXactCallback(XactCallback callback, void *arg);
--- 244,251 ----
extern char TransactionBlockStatusCode(void);
extern void AbortOutOfAnyTransaction(void);
extern void PreventTransactionChain(bool isTopLevel, const char *stmtType);
! extern void RequireTransactionChain(bool isTopLevel, bool throwError,
! 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
