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@roji.org> wrote:
> > As Dave wrote, the problem here isn't with the driver, but with framework or user-code which swallows the initial
exceptionand allows code to continue to the commit. Npgsql (and I'm sure the JDBC driver too) does surface PostgreSQL
errorsas exceptions, and internally tracks the transaction status provided in the CommandComplete message. That means
usershave the ability - but not the obligation - to know about failed transactions, and some frameworks or user coding
patternscould 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
Iwrote above, from the user's perspective it makes no difference - the change would be identical (and just as breaking)
eitherway. 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
aproblem 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
transactionstate etc.), then making this change at the server wouldn't affect those applications; the only applications
affectedwould be those that do commit on failed transactions today, and it could be argued that those are likely to be
brokentoday (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