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

On Mon, 2021-01-25 at 11:29 -0500, Dave Cramer wrote:
> Rebased against head 
> 
> Here's my summary of the long thread above.
> 
> This change is in keeping with the SQL spec.
> 
> There is an argument (Tom) that says that this will annoy more people than it will please.
>  I presume this is due to the fact that libpq behaviour will change.
> 
> As the author of the JDBC driver, and I believe I speak for other driver (NPGSQL for one)
>  authors as well that have implemented the protocol I would argue that the current behaviour
>  is more annoying.
> 
> We currently have to keep state and determine if COMMIT actually failed or it ROLLED BACK.
>  There are a number of async drivers that would also benefit from not having to keep state
>  in the session.

I think this change makes sense, but I think everybody agrees that it does as it
makes PostgreSQL more standard compliant.

About the fear that it will break user's applications:

I think that the breakage will be minimal.  All that will change is that COMMIT of
an aborted transaction raises an error.

Applications that catch an error in a transaction and roll back will not
be affected.  What will be affected are applications that do *not* check for
errors in statements in a transaction, but check for errors in the COMMIT.
I think that doesn't happen often.

I agree that some people will be hurt, but I don't think it will be a major problem.

The patch applies and passes regression tests.

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.

Is this new message level something we need to allow setting for
"client_min_messages" and "log_min_messages"?

Yours,
Laurenz Albe