DERBY-966 - holdability - dubious code in network server.

[Daniel John Debrunner <dj...@apache.org>]

I've think I've found a dubious piece of code that could be the cause of
DERBY-966, holdability issues with XA connections and Derby client.
This is within the server code.

lines 513 DRDAStatement.java

Connection conn = database.getConnection();
if (conn instanceof BrokeredConnection)
   ps = conn.prepareStatement(sqlStmt, scrollType, concurType);
else
  ps = prepareStatementJDBC3(sqlStmt, scrollType, concurType,
        withHoldCursor);


The 'if (conn instanceof BrokeredConnection)' is dubious. For one it
doesn't have comments, so I have to guess what the original coder was
trying to do. (one more plug for code comments :-)

>From the svn log it looks like this is really meant to be logic of the
form, if running under jdk 13 do the first statement, else do the second
statement. The bug is that the instanceof check is not an absolute class
check, it is an instance of check, when we have an XA connection the
connection object will be a BrokeredConnection or BrokeredConnection30.
Since BrokeredConnection30 extends BrokeredConnection both of them are
instances of BrokeredConnection, so the instanceof will always be true
and so it looks like we lose holdability state in JDBC 3.0 and above.

I'm still looking through the code to understand how the holdability is
meant to work in jdk 13 and the network server, any pointers on that
would be helpful.

Dan.

Re: DERBY-966 - holdability - dubious code in network server.

[Kathey Marsden <km...@sbcglobal.net>]

Daniel John Debrunner wrote:

>I've think I've found a dubious piece of code that could be the cause of
>DERBY-966, holdability issues with XA connections and Derby client.
>This is within the server code.
>
>lines 513 DRDAStatement.java
>
>Connection conn = database.getConnection();
>if (conn instanceof BrokeredConnection)
>   ps = conn.prepareStatement(sqlStmt, scrollType, concurType);
>else
>  ps = prepareStatementJDBC3(sqlStmt, scrollType, concurType,
>        withHoldCursor);
>
>
>The 'if (conn instanceof BrokeredConnection)' is dubious.
>  
>
Yes, I think you are right . Not only dubious but badly broken as
evidenced by DERBY-966 . DERBY-1005, and DERBY-1006.

The wrong assumptions here (if  I recall) were
1) That a BrokeredConnection meant we were in an XA transaction
2) For XA connections we  could rely on the  embedded server's process
of switching to CLOSE_CURSORS_AT_COMMIT to avoid having such logic on
the client.

>I'm still looking through the code to understand how the holdability is
>meant to work in jdk 13 and the network server, any pointers on that
>would be helpful.
>
>  
>
In general the way  it works  for both jdk14 and jdk13 is that the
holdability is sent by the client in the PKGNAMCSN.
This is parsed in  parsePkgidToFindHoldability() to determine the
holdability.   Then statements are prepared using that value in
prepareStatementJDBC3.

For XA all of this is summarly ignored per the code above.  Now it seems
quite transparent that that is wrong, but oddly it seemed to make sense
at the time.

Kathey