Re: Error on failed COMMIT

On Tue, 17 Mar 2020 at 16:47, Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> On Fri, Mar 6, 2020 at 01:12:10PM -0500, Robert Haas wrote:
> > On Fri, Mar 6, 2020 at 11:55 AM Dave Cramer <davecramer(at)postgres(dot)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 parse the SQL to figure
> out what is being sent. This could include a column named commit_date or a
> comment with commit embedded in it. It really doesn't make sense to have a
> full fledged PostgreSQL SQL parser in every client. This is something the
> 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.
>
> Let me try to summarize where I think we are on this topic.
>
> First, Vik reported that we don't follow the SQL spec when issuing a
> COMMIT WORK in a failed transaction. We return success and issue the
> ROLLBACK command tag, rather than erroring. In general, if we don't
> follow the spec, we should either have a good reason, or the breakage to
> match the spec is too severe. (I am confused why this has not been
> reported before.)
>

Good question.

>
> Second, someone suggested that if COMMIT throws an error, that future
> statements would be considered to be in the same transaction block until
> ROLLBACK is issued. It was determined that this is not required, and
> that the API should have COMMIT WORK on a failed transaction still exit
> the transaction block. This behavior is much more friendly for SQL
> scripts piped into psql.
>
> This is correct. The patch I provided does exactly this.
The Rollback occurs. The transaction is finished, but an error message is
sent

> Third, the idea that individual interfaces, e.g. JDBC, should throw an
> error in this case while the server just changes the COMMIT return tag
> to ROLLBACK is confusing. People regularly test SQL commands in the
> server before writing applications or while debugging, and a behavior
> mismatch would cause confusion.
>

I'm not sure what you mean by this. The server would throw an error.

>
> Fourth, it is not clear how many applications would break if COMMIT
> started issuing an error rather than return success a with ROLLBACK tag.
> Certainly SQL scripts would be fine. They would have one additional
> error in the script output, but if they had ON_ERROR_STOP enabled, they
> would have existed before the commit. Applications that track statement
> errors and issue rollbacks will be fine. So, we are left with
> applications that issue COMMIT and expect success after a transaction
> block has failed. Do we know how other database systems handle this?
>

Well I know pgjdbc handles my patch fine without any changes to the code
As I mentioned upthread 2 of the 3 go drivers already error if rollback is
returned. 1 of them does not.

I suspect npgsql would be fine. Shay ?

Dave Cramer
www.postgres.rocks