Re: Error on failed COMMIT

On Fri, Mar 6, 2020 at 11:55 AM Dave Cramer <davecramer@postgres.rocks> wrote:
> There have been some arguments that the client can fix this easily.
>
> Turns out it is not as easy as one might think.
>
> If the client (in this case JDBC) uses conn.commit() then yes relatively easy as we know that commit is being
executed.

Right...

> however if the client executes commit using direct SQL and possibly multiplexes a number of commands we would have to
parsethe SQL to figure out what is being sent. This could include a column named commit_date or a comment with commit
embeddedin it. It really doesn't make sense to have a full fledged PostgreSQL SQL parser in every client. This is
somethingthe server does very well. 

That's true. If the command tag is either COMMIT or ROLLBACK then the
statement was either COMMIT or ROLLBACK, but Vladimir's example query
/*commit*/rollback does seem like a pretty annoying case. I was
assuming that the JDBC driver required use of con.commit() in the
cases we care about, but perhaps that's not so.

> There has been another argument that we can simply check the transaction state after we get the ReadyForQuery
response,however this is set to IDLE after the subsequent ROLLBACK  so that doesn't work either. 

I assumed you'd look at the *previous* ReadyForQuery message and see
whether it said "in transaction" ('T') or "failed in transaction"
('E'). If the transaction was failed, then only rollback is possible,
but if it's not, then either commit or rollback is possible.

But I agree that if you don't know what you command you sent, and have
to deal with users who send things like /*commit*/rollback, then the
current reporting is not good enough. If the command tag for a commit
that got converted into a rollback were distinct from the command tag
that you get from a deliberate rollback, then it would be file; say if
we sent ROLLBACK COMMIT for one and just ROLLBACK for the other, for
example. But that's not how it works.

I think you can still fix the con.commit() case. But users issuing
ad-hoc SQL that may contain comments intended to snipe the driver
seems like it does require a server-side change.

> Additionally in section 52.2.2 of the docs it states:
>
> A frontend must be prepared to accept ErrorResponse and NoticeResponse messages whenever it is expecting any other
typeof message. See also Section 52.2.6 concerning messages that the backend might generate due to outside events. 
>
> Recommended practice is to code frontends in a state-machine style that will accept any message type at any time that
itcould make sense, rather than wiring in assumptions about the exact sequence of messages. 
>
> Seems to me that this behaviour is already documented?

I don't understand what you're going for here.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company