Re: Error on failed COMMIT - Mailing list pgsql-hackers

On Tue, 2021-01-26 at 11:09 -0500, Dave Cramer wrote:
> On Tue, 26 Jan 2021 at 05:05, Laurenz Albe <laurenz.albe@cybertec.at> wrote:
>
> > I wonder about the introduction of the new USER_ERROR level:
> >
> >  #define WARNING_CLIENT_ONLY    20  /* Warnings to be sent to client as usual, but
> >                                  * never to the server log. */
> > -#define ERROR      21          /* user error - abort transaction; return to
> > +#define USER_ERROR 21
> > +#define ERROR      22          /* user error - abort transaction; return to
> >                                  * known state */
> >  /* Save ERROR value in PGERROR so it can be restored when Win32 includes
> >   * modify it.  We have to use a constant rather than ERROR because macros
> >   * are expanded only when referenced outside macros.
> >   */
> >  #ifdef WIN32
> > -#define PGERROR        21
> > +#define PGERROR        22
> >  #endif
> > -#define FATAL      22          /* fatal error - abort process */
> > -#define PANIC      23          /* take down the other backends with me */
> > +#define FATAL      23          /* fatal error - abort process */
> > +#define PANIC      24          /* take down the other backends with me */
> >
> > I see that without that, COMMIT AND CHAIN does not behave correctly,
> > since the respective regression tests fail.
> >
> > But I don't understand why.  I think that this needs some more comments to
> > make this clear.
>
> First off thanks for reviewing.
>
> The problem is that ereport does not return for any level equal to or above ERROR.
>  This code required it to return so that it could continue processing

Oh, I see.

After thinking some more about it, I think that COMMIT AND CHAIN would have
to change behavior: if COMMIT throws an error (because the transaction was
aborted), no new transaction should be started.  Everything else seems fishy:
the statement fails, but still starts a new transaction?

I guess that's also at fault for the unexpected result status that
Masahiko complained about in the other message.

So I think we should not introduce USER_ERROR at all.  It is too much
of a kluge: fail, but not really...