From: | Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> |
---|---|
To: | Dave Cramer <davecramer(at)postgres(dot)rocks>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> |
Cc: | Tony Locke <tlocke(at)tlocke(dot)org(dot)uk>, Shay Rojansky <roji(at)roji(dot)org>, Bruce Momjian <bruce(at)momjian(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Vik Fearing <vik(at)postgresfriends(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Vladimir Sitnikov <sitnikov(dot)vladimir(at)gmail(dot)com>, "Haumacher, Bernhard" <haui(at)haumacher(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Error on failed COMMIT |
Date: | 2021-01-26 10:05:53 |
Message-ID: | 77420b93cdce9992efcea05e91254b928293a492.camel@cybertec.at |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Heikki Linnakangas | 2021-01-26 10:23:29 | Re: automatic analyze: readahead - add "IO read time" log message |
Previous Message | Takashi Menjo | 2021-01-26 09:50:50 | Re: [PoC] Non-volatile WAL buffer |