Re: PG_RE_THROW is mandatory (was Re: jsonpath)

On 2019-02-06 13:09:59 -0300, Alvaro Herrera wrote:
> In https://postgr.es/m/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