Re: Error on failed COMMIT

Laurenz Albe <laurenz.albe@cybertec.at>

From: Laurenz Albe <laurenz.albe@cybertec.at>

To: Dave Cramer <davecramer@postgres.rocks>

Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>, Tony Locke <tlocke@tlocke.org.uk>, Shay Rojansky <roji@roji.org>, Bruce Momjian <bruce@momjian.us>, Robert Haas <robertmhaas@gmail.com>, Vik Fearing <vik@postgresfriends.org>, Tom Lane <tgl@sss.pgh.pa.us>, Vladimir Sitnikov <sitnikov.vladimir@gmail.com>, "Haumacher, Bernhard" <haui@haumacher.de>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>

Date: 2021-01-26T17:20:41Z

Lists: 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...

I guess that is one example for the incompatibilities that Tom worried
about upthread.  I am beginning to see his point better now.

Yours,
Laurenz Albe