Re: Error on failed COMMIT

From: Dave Cramer <davecramer(at)postgres(dot)rocks>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Shay Rojansky <roji(at)roji(dot)org>, "Haumacher, Bernhard" <haui(at)haumacher(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Error on failed COMMIT
Date: 2020-02-24 13:16:11
Message-ID: CADK3HH+gdbRdaqm6tyu+smU7N4Oijw3QUjbm6gQhMMJwj_XfXQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, 24 Feb 2020 at 07:34, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Feb 24, 2020 at 1:56 PM Shay Rojansky <roji(at)roji(dot)org> wrote:
> > As Dave wrote, the problem here isn't with the driver, but with
> framework or user-code which swallows the initial exception and allows code
> to continue to the commit. Npgsql (and I'm sure the JDBC driver too) does
> surface PostgreSQL errors as exceptions, and internally tracks the
> transaction status provided in the CommandComplete message. That means
> users have the ability - but not the obligation - to know about failed
> transactions, and some frameworks or user coding patterns could lead to a
> commit being done on a failed transaction.
>
> Agreed. All of that can be fixed in the driver, though.
>

Of course it can but we really don't want our users getting one experience
with driver A and a different experience with driver B.

>
> > If we think the current *user-visible* behavior is problematic (commit
> on failed transaction completes without throwing), then the only remaining
> question is where this behavior should be fixed - at the server or at the
> driver. As I wrote above, from the user's perspective it makes no
> difference - the change would be identical (and just as breaking) either
> way. So while drivers *could* implement the new behavior, what advantages
> would that have over doing it at the server? Some disadvantages do seem
> clear (repetition of the logic across each driver - leading to
> inconsistency across drivers, changing semantics at the driver by turning a
> non-error into an exception...).
>
> The advantage is that it doesn't cause a compatibility break.
>

Sure it does. Any existing code that was relying on the existing semantics
would be incompatible.

>
> > > Well, it seems quite possible that there are drivers and applications
> that don't have this issue; I've never had a problem with this behavior,
> and I've been using PostgreSQL for something like two decades [...]
> >
> > If we are assuming that most user code is already written to avoid
> committing on failed transactions (by tracking transaction state etc.),
> then making this change at the server wouldn't affect those applications;
> the only applications affected would be those that do commit on failed
> transactions today, and it could be argued that those are likely to be
> broken today (since drivers today don't really expose the rollback in an
> accessible/discoverable way).
>
> libpq exposes it just fine, so I think you're overgeneralizing here.
>
> As I said upthread, I think one of the things that would be pretty
> badly broken by this is psql -f something.sql, where something.sql
> contains a series of blocks of the form "begin; something; something;
> something; commit;". Right now whichever transactions succeed get
> committed. With the proposed change, if one transaction block fails,
> it'll merge with all of the following blocks.

So how does one figure out what failed and what succeeded ? I would think
it would be pretty difficult in a large sql script to go back and figure
out what needed to be repaired. Seems to me it would be much easier if
everything failed.

> You may think that
> nobody is doing this sort of thing, but I think people are, and that
> they will come after us with pitchforks if we break it.
>

So the argument here is that we don't want to annoy some percentage of the
population by doing the right thing ?

Dave Cramer
www.postgres.rocks

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Dave Cramer 2020-02-24 13:21:48 Re: Error on failed COMMIT
Previous Message Amit Langote 2020-02-24 13:10:56 Re: v12 "won't fix" item regarding memory leak in "ATTACH PARTITION without AEL"; (or, relcache ref counting)