On Tue, Nov 19, 2013 at 10:21:47PM -0500, Tom Lane wrote:
> Bruce Momjian <br...@momjian.us> writes:
> > Does anyone know if this C comment justifies why ABORT is a NOTICE and
> > not WARNING?
>
> > /*
> > * 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.
> > */
>
> It's just describing the implementation, not defending the design choice.
>
> My personal standpoint is that I don't care much whether these messages
> are NOTICE or WARNING. What I'm not happy about is promoting cases that
> have been non-error conditions for years into ERRORs.
I don't remember any cases where that was suggested.
> Now, if we wanted to go the other way (downgrade some ERRORs to WARNINGs),
> that would not create an application compatibility problem in my view.
Yes, that was my initial plan, but many didn't like it.
> And on the third hand, there's Emerson's excellent advice: "A foolish
> consistency is the hobgoblin of little minds". I'm not convinced that
> trying to make all these cases have the same message level is actually
> a good goal. It seems entirely plausible that some are more dangerous
> than others.
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.
Interesting that ABORT was already documented as returning a warning:
Issuing <command>ABORT</> when not inside a transaction does
no harm, but it will provoke a warning message.
-------
--
Bruce Momjian <br...@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
new file mode 100644
index 0591f3f..495684e
*** a/src/backend/access/transam/xact.c
--- b/src/backend/access/transam/xact.c
*************** PreventTransactionChain(bool isTopLevel,
*** 2961,2966 ****
--- 2961,2969 ----
* 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)
+ * outside of transactions issue warnings, these generate errors.
+ *
* isTopLevel: passed down from ProcessUtility to determine whether we are
* inside a function.
* stmtType: statement type name, for error messages.
*************** 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;
--- 3428,3439 ----
/*
* 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/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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
