Re: Error on failed COMMIT

On Sun, 23 Feb 2020 at 20:31, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Sun, Feb 23, 2020 at 11:11 AM Shay Rojansky <roji(at)roji(dot)org> wrote:
> > I'd like to second Dave on this, from the .NET perspective - actual
> client access is done via standard drivers in almost all cases, and these
> drivers generally adhere to database API abstractions (JDBC for Java,
> ADO.NET for .NET, and so on). AFAIK, in almost all such abstractions,
> commit can either complete (implying success) or throw an exception - there
> is no third way to return a status code. It's true that a driver may expose
> NOTICE/WARNING messages via some other channel (Npgsql emits .NET events
> for these), but this is a separate message "channel" that is disconnected
> API-wise from the commit; this makes the mechanism very "undiscoverable".
>
> I'm still befuddled here. First, to repeat what I said before, the
> COMMIT only returns a ROLLBACK command tag if there's been a previous
> ERROR. So, if you haven't ignored the prior ERROR, you should be fine.
> Second, there's nothing to keep the driver itself from translating
> ROLLBACK into an exception, if that's more convenient for some
> particular driver. Let's go back to Bernhard's example upthred:
>
> composeTransaction() {
> Connection con = getConnection(); // implicitly "begin"
> try {
> insertFrameworkLevelState(con);
> insertApplicationLevelState(con);
> con.commit();
> publishNewState();
> } catch (Throwable ex) {
> con.rollback();
> }
> }
>
> If insertFrameworkLevelState() or insertApplicationLevelState()
> perform database operations that fail, then an exception should be
> thrown and we should end up at con.rollback(), unless there is an
> internal catch block inside those functions that swallows the
> exception, or unless the JDBC driver ignores the error from the
> server.

The driver does not ignore the error, But in Bernhard's case the framework
is
processing the exception and not re-throwing it.

> If those things succeed, then COMMIT could still fail with an
> ERROR but it shouldn't return ROLLBACK. But, for extra security,
> con.commit() could be made to throw an exception if the command tag
> returned by COMMIT is not COMMIT. It sounds like Dave doesn't want to
> do that, but it would solve this problem without requiring a server
> behavior change.
>

Well the driver really isn't in the business of changing the semantics of
the server.

>
> Actually, an even better idea might be to make the driver error out
> when the transaction is known to be in a failed state when you enter
> con.commit(). The server does return an indication after each command
> as to whether the session is in a transaction and whether that
> transaction is in a failed state. That's how the %x escape sequence
> just added to the psql prompt works. So, suppose the JDBC driver
> tracked that state like libpq does. insertFrameworkLevelState() or
> insertApplicationLevelState() throws an exception, which is internally
> swallowed. Then you reach con.commit(), and it says, nope, can't do
> that, we're in a failed state, and so an exception is thrown. Then
> when we reach con.rollback() we're still inside a transaction, it gets
> rolled back, and everything works just as expected.
>

Yes, we could do that.

>
> Or, alternatively, the JDBC driver could keep track of the fact that
> it had thrown an exception ITSELF, without paying any attention to
> what the server told it, and if it saw con.commit() after raising an
> exception, it could raise another exception (or re-raise the same
> one). That would also fix it.
>

We could also do this.

>
> > Asking drivers to do this at the client have the exact same breakage
> impact as the server change, since the user-visible behavior changes in the
> same way (the change is just shifted from server to driver). What's worse
> is that every driver now has to reimplement the same new logic, and we'd
> most probably end up with some drivers doing it in some languages, and
> others not doing it in others (so behavioral differences).
>
> 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,

I would argue that you really don't know if you had the problem or not
since an error is not thrown.
The client merrily goes along its way after issuing a commit and receiving
a rollback or possibly a warning
saying that it's not in a transaction. One would have to know that the
server
had this behaviour to check for it.
Clearly not everyone knows this as it's not documented as violation of the
SQL spec

> and I believe that the sketch above could be used to get the
> desired behavior in current releases of PostgreSQL with no server code
> change. If we did change the server behavior, it seems unlikely that
> every driver would adjust their behavior to the new server behavior
> all at once and that they would all get it right while also all
> preserving backward compatibility with current releases in case a
> newer driver is used with an older server.

Actually I would be willing to bet that the JDBC driver would do just that
without any code changes whatsoever.
commit throws an error. The driver sees the error and throws an exception.
If an older version of the server were used no error would be thrown it
wold work as seen today. I would also be willing to bet other drivers would
work the same.
Additionally once the server behaviour was changed I'd be more than willing
to have the driver emulate this behaviour for older versions.

> I don't think that's
> likely. What would probably happen is that many drivers would ignore
> the change, leaving applications to cope with the differences between
> server versions, and some would change the driver behavior
> categorically, breaking compatibility with older server versions, and
> some would make mistakes in implementing support for the new behavior.
> And maybe we would also find that the new behavior isn't ideal for
> everybody any more than the current behavior is ideal for everybody.
>
> I am really struggling to see why this is anything but a bug in the
> JDBC driver.

Not seeing how this is a driver error.

> The problem is that the application doesn't know that the
> transaction has failed, but the server has returned not one, not two,
> but three indications of failure. First, it returned an error, which I
> guess the JDBC driver turns into an exception - but it does not,
>
It does throw the exception, however for whatever reason the client ignores
these.
Not how I code but apparently there is an application for this

> before throwing that exception, remember that the current transaction
> is failed. Second, it will thereafter report that the transaction is
> in a failed state, both immediately after the error and upon every
> subsequent operation that does not get the server out of the
> transaction. It sounds like the JDBC driver ignores this information.
>

> Third, the attempt at COMMIT will return a ROLLBACK command tag, which
> Dave said that the driver does ignore. That's a lot of stuff that the
> driver could do but isn't doing. So what this boils down to, from my
> perspective, is not that the driver behavior in the face of errors
> can't be made correct with the existing semantics, but that the driver
> would find it more convenient if PostgreSQL reported those errors in a
> somewhat different way. I think that's a fair criticism, but I don't
> think it's a sufficient reason to change the behavior.
>

I think the fact that this is a violation of the SQL SPEC lends
considerable credence to the argument for changing the behaviour.
Since this can lead to losing a transaction I think there is even more
reason to look at changing the behaviour.

Dave Cramer
www.postgres.rocks