Re: Error on failed COMMIT

On Mon, Feb 24, 2020 at 06:04:28PM +0530, Robert Haas 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.
>
> > 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.
>
> > > 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. 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.

I'm doing it, and I don't know about pitchforks, but I do know about
suddenly needing to rewrite (and re-test, and re-integrate, and
re-test some more) load-bearing code, and I'm not a fan of it.

If we'd done this from a clean sheet of paper, it would have been the
right decision. We're not there, and haven't been for decades.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate