PG_RE_THROW is mandatory (was Re: jsonpath)

On 2019-02-06 13:09:59 -0300, Alvaro Herrera wrote:

In /messages/by-id/1676.1548726280@sss.pgh.pa.us Tom Lane wrote:

Sure: every errcode we have is unsafe to treat this way.

The backend coding rule from day one has been that a thrown error requires
(sub)transaction cleanup to be done to make sure that things are back in a
good state. You can *not* just decide that it's okay to ignore that,
especially not when invoking code outside the immediate area of what
you're doing.

elog.h claims that PG_RE_THROW is "optional":

/*----------
* API for catching ereport(ERROR) exits. Use these macros like so:
*
* PG_TRY();
* {
* ... code that might throw ereport(ERROR) ...
* }
* PG_CATCH();
* {
* ... error recovery code ...
* }
* PG_END_TRY();
*
* (The braces are not actually necessary, but are recommended so that
* pgindent will indent the construct nicely.) The error recovery code
* can optionally do PG_RE_THROW() to propagate the same error outwards.

This is obviously wrong; while we have a couple of codesites that omit
it, it's not a generally available coding pattern. I think we should
amend that comment. I propose: "The error recovery code must normally
do PG_RE_THROW() to propagate the error outwards; failure to do so may
leave the system in an inconsistent state for further processing."

Well, but it's ok not to rethrow if you do a [sub]transaction
rollback. I assume that's why it's framed as optional. We probably
should reference that fact?

Greetings,

Andres Freund

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

elog.h claims that PG_RE_THROW is "optional":

* (The braces are not actually necessary, but are recommended so that
* pgindent will indent the construct nicely.) The error recovery code
* can optionally do PG_RE_THROW() to propagate the same error outwards.

This is obviously wrong; while we have a couple of codesites that omit
it, it's not a generally available coding pattern. I think we should
amend that comment. I propose: "The error recovery code must normally
do PG_RE_THROW() to propagate the error outwards; failure to do so may
leave the system in an inconsistent state for further processing."

Well, it can either do PG_RE_THROW or do a (sub)transaction abort.
Some level of throw-catching code has to do the latter eventually.

regards, tom lane

On 2019-02-06 13:09:59 -0300, Alvaro Herrera wrote:

This is obviously wrong; while we have a couple of codesites that omit
it, it's not a generally available coding pattern. I think we should
amend that comment. I propose: "The error recovery code must normally
do PG_RE_THROW() to propagate the error outwards; failure to do so may
leave the system in an inconsistent state for further processing."

On 2019-Feb-06, Andres Freund wrote:

Well, but it's ok not to rethrow if you do a [sub]transaction
rollback. I assume that's why it's framed as optional. We probably
should reference that fact?

On 2019-Feb-06, Tom Lane wrote:

Well, it can either do PG_RE_THROW or do a (sub)transaction abort.
Some level of throw-catching code has to do the latter eventually.

So,

"The error recovery code can either do PG_RE_THROW to propagate the
error outwards, or do a (sub)transaction abort. Failure to do so may
leave the system in an inconsistent state for further processing."

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

So,

"The error recovery code can either do PG_RE_THROW to propagate the
error outwards, or do a (sub)transaction abort. Failure to do so may
leave the system in an inconsistent state for further processing."

WFM.

regards, tom lane

On 2019-Feb-07, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

So,

"The error recovery code can either do PG_RE_THROW to propagate the
error outwards, or do a (sub)transaction abort. Failure to do so may
leave the system in an inconsistent state for further processing."

WFM.

Thanks, pushed.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services