Re: jdbc xa patches

On Wed, 27 Jul 2005, Oliver Jowett wrote:

> Michael Allman wrote:
>> On Wed, 27 Jul 2005, Oliver Jowett wrote:
>>
>>> We now use 4-space indents, not tabs.
>>
>> I'm away from my normal dev machine and am flailing away with unfamiliar
>> tools.  Do you have a utility that will indent as desired?
>
> perl? :-)

ugh :-(

>>> Shouldn't the XAResource check the server version on construction or on
>>> start()/recover() to make sure that it's actually going to be able to
>>> use PREPARE TRANSACTION later? Or is erroring out with a syntax error at
>>> the point of prepare() sufficient? (I'd like to see a better error
>>> message there at least)
>>
>> If we call statement.executeUpdate("SOME QUERY POSTGRESQL DOESN'T
>> UNDERSTAND") will it throw an instance of SQLException?
>
> Yes, per JDBC spec. Still, it is generally tricky to distinguish a particular
> error class from more general errors (SQLSTATE only gets you so far).

First, I'm assuming that any general release of the JDBC driver with XA
support will make it clear up front that it requires Postgres 8.1.

With that in mind, if some mortal is foolish enough to attempt to prepare
a transaction on an unsupported backend, executeUpdate() will barf up an
SQLException, which will cause PGXAResource to throw an XAException yada
yada yada which will cause the TM to rollback the transaction.  So at
least they won't be in Dangling Transaction Hell.

Anyway, I'm counting on Postgres users knowing what they're doing, esp.
with something as non-trivial as XA support.

>>> Having ResourceAssociationState as a full-blown class seems like
>>> overkill since you only have one instance anyway.
>>
>> Maybe.  Java 1.4 doesn't have enums.  What do you suggest as an
>> alternative?
>
> Just a bare int?

In this kind of situation I prefer an enum or enum-like class to an int.
I don't dispute that it would work with just an int.  As a matter of
personal preference I like to use a more specific type.

>>> Is XID_TO_TRANSACTION_STATE_MAP actually necessary? It seems like it's
>>> only there for detecting XA protocol errors (which would otherwise show
>>> up as errors from the backend, e.g. trying to commit a nonexistant
>>> transaction), and for tracking the state of the transaction that is
>>> currently suspended or currently associated (of which there can only be
>>> one in the current implementation)
>>
>> In the implementation of rollback() it is used to determine whether to call
>> physicalConnection.rollback() or issue a "ROLLBACK PREPARED" query.
>
> Isn't that covered by tracking the state of the one transaction that's
> currently in progress on the connection? i.e. if you get asked to rollback()
> while it's active, you do a rollback(); otherwise you do a ROLLBACK PREPARED.

I think you're right.  I also think this is a matter of style/personal
preference.  I like to give classes some of the responsibility for their
correct usage.  It makes it easier to diagnose problems when a class
instance is used incorrectly.

Michael