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