Re: prepare() method error

Hi all,

I've been testing the XA implementation and found a bug. The
PGXAConnection prepare(Xid) method may return a value of XA_OK when it
should throw an XAException instead. This can lead to an inconsistent TPC
outcome.

For example, if you attempt to insert a row which violates a uniqueness
constraint and then prepare the transaction branch, the prepare() method
returns XA_OK erroneously because the PREPARE TRANSACTION statement
executes without throwing an exception even though the transaction is not
prepared.

As a workaround, I've added a query after the PREPARE TRANSACTION query in
the prepare() method which searches the pg_prepared_xacts table for a row
with the given gid just "prepared". If it doesn't find the corresponding
row, it throws an XAException with a suitable error code (I chose
XA_RBROLLBACK).

It would be cleaner if the driver could determine the success/failure of
the PREPARE TRANSACTION statement from the query itself. For instance,
have the executeUpdate("PREPARE TRANSACTION " ....) return normally if and
only if it actually prepares a transaction. If it doesn't prepare, throw
a meaningful SQLException.

Michael

On Sun, 12 Feb 2006, Michael Allman wrote:

> Hi all,
>
> I've been testing the XA implementation and found a bug. The
> PGXAConnection prepare(Xid) method may return a value of XA_OK when it
> should throw an XAException instead. This can lead to an inconsistent TPC
> outcome.
>
> For example, if you attempt to insert a row which violates a uniqueness
> constraint and then prepare the transaction branch, the prepare() method
> returns XA_OK erroneously because the PREPARE TRANSACTION statement
> executes without throwing an exception even though the transaction is not
> prepared.

Good point.

> As a workaround, I've added a query after the PREPARE TRANSACTION query in
> the prepare() method which searches the pg_prepared_xacts table for a row
> with the given gid just "prepared". If it doesn't find the corresponding
> row, it throws an XAException with a suitable error code (I chose
> XA_RBROLLBACK).
>
> It would be cleaner if the driver could determine the success/failure of
> the PREPARE TRANSACTION statement from the query itself. For instance,
> have the executeUpdate("PREPARE TRANSACTION " ....) return normally if and
> only if it actually prepares a transaction. If it doesn't prepare, throw
> a meaningful SQLException.

We can check the command status string. PREPARE TRANSACTION should
return "PREPARE TRANSACTION" command status on success and "ROLLBACK" on
failure.

- Heikki

On Sun, 12 Feb 2006, Heikki Linnakangas wrote:

> On Sun, 12 Feb 2006, Michael Allman wrote:
>>
>> It would be cleaner if the driver could determine the success/failure of
>> the PREPARE TRANSACTION statement from the query itself. For instance,
>> have the executeUpdate("PREPARE TRANSACTION " ....) return normally if and
>> only if it actually prepares a transaction. If it doesn't prepare, throw a
>> meaningful SQLException.
>
> We can check the command status string. PREPARE TRANSACTION should return
> "PREPARE TRANSACTION" command status on success and "ROLLBACK" on failure.

And that's what I did, see attached patch. I had to add quite a few
imports to PGXAConnection so that it can work on the lower level and check
the command status coming from the server.

Michael: Does it now look OK to you?

It just occured to me that we have the same problem with regular commit,
don't we? If the transaction has failed, commit doesn't throw an
exception. I'm not sure if it should. The java.sql javadoc doesn't
explicitly say so, but IMHO it should.

- Heikki