This patch is committed. Thanks for updating ALL versions of the master file.
What happens if getCharacterStream() or getBinaryStream() is used instead of getClob() or getBlob()? Satheesh Sending java\engine\org\apache\derby\impl\jdbc\EmbedBlob.java Sending java\engine\org\apache\derby\impl\jdbc\EmbedClob.java Sending java\engine\org\apache\derby\impl\store\raw\data\BaseContainerHandle.java Sending java\engine\org\apache\derby\impl\store\raw\data\OverflowInputStream.java Sending java\testing\org\apache\derbyTesting\functionTests\master\DerbyNet\blobclob4BLOB.out Sending java\testing\org\apache\derbyTesting\functionTests\master\DerbyNetClient\blobclob4BLOB.out Sending java\testing\org\apache\derbyTesting\functionTests\master\blobclob4BLOB.out Sending java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\blobclob4BLOB.java Transmitting file data ........ Committed revision 178803. [bandaram:satheesh] Mike Matrigali wrote: > I've looked at the diffs and changes look good to me. Could one of the > committers with jdbc expertise pick this one up and make sure that part > of the change is right. The raw store is fine. > > Sunitha Kambhampati wrote: > >> This patch is a fix for Derby -265. >> http://issues.apache.org/jira/browse/DERBY-265 - In Network Server >> retrieving BLOB values with autocommit off causes >> NullPointerException in INSANE build / AssertFailure in >> BaseContainerHandle.getTransaction in SANE Build >> >> >> The problem >> -- Basically, in autocommit mode, when getBlob is called on a >> resultset after the transaction in which it was created is committed >> throws an NPE. Per the jdbc api and spec, getBlob is valid only for >> the duration of the transaction in which it was created. So it is >> incorrect to call getBlob as in this repro for derby-265. >> -- On a getBlob for overflow columns, we initiliaze the stream by >> reopening the container. In here, the transaction of the >> containerhandle ends up being null and an NPE is thrown. >> -- The problem is not specific to network server as such, but is >> reproducible in embedded mode also. >> >> >> Fix includes >> -- Adds check in OverflowInputStream.initStream to see if >> transaction of the container handle is null and throws a >> StandardException with SQLState.DATA_CONTAINER_CLOSED >> -- And at the jdbc layer, this exception is wrapped with a user >> exception with an existing SQLState XJ073 >> (SQLState.BLOB_ACCESSED_AFTER_COMMIT) for both getBlob and getClob. >> The error message corresponding to this sqlstate is "The data in >> this Blob or Clob is no longer available. Possible reasons are that >> its transaction committed, or its connection closed." >> -- Removed the ASSERT in BaseContainerHandle.getTransaction() >> >> Also note, if you try to do a read on the blob when transaction >> closes (in autocommit mode), we already handle such cases and throw a >> similar error(either container is closed or that the data in blob or >> clob is no longer available )depending on the api used. >> >> Testing >> -- Added test cases to existing jdbcapi/blobclob4BLOB.java , this >> test will run in both network server and embedded mode >> -- Ran derbyall on jdk142/Win2k. >> All tests passed except for one test that currently fails with >> DerbyNetClient - lang/updatableResultSet.java. The diff seems to be >> difference in cursor names. This test also fails on a clean >> build(without my changes). >> svn stat >> M java\engine\org\apache\derby\impl\jdbc\EmbedBlob.java >> M java\engine\org\apache\derby\impl\jdbc\EmbedClob.java >> M >> java\engine\org\apache\derby\impl\store\raw\data\OverflowInputStream.java >> >> M >> java\engine\org\apache\derby\impl\store\raw\data\BaseContainerHandle.java >> >> M >> java\testing\org\apache\derbyTesting\functionTests\tests\jdbcapi\blobclob4BLOB.java >> >> M >> java\testing\org\apache\derbyTesting\functionTests\master\DerbyNet\blobclob4BLOB.out >> >> M >> java\testing\org\apache\derbyTesting\functionTests\master\DerbyNetClient\blobclob4BLOB.out >> >> M >> java\testing\org\apache\derbyTesting\functionTests\master\blobclob4BLOB.out >> >> >> Can someone review it, and if there are no comments can a committer >> please commit the patch. >> >> Thanks, >> Sunitha. >> >> >> Mike Matrigali wrote: >> >>> I think your description below is valid. Blob's are not valid after >>> the >>> transaction that opened them commits. Doing interleaved result sets in >>> autocommit mode is almost always a bug waiting to happen. "Held" >>> cursors help some, but still after the commit you must do a next - the >>> current blob is not valid. >>> >>> A NPE is not a good error, but if possible I would like to see the >>> catch of the error condition pushed as high up in the code as possible. >>> Is it possible for the jdbc getBlob() call to recognize that the >>> transaction of the blob has closed? If not then maybe at least the >>> catch can be placed in the blob datatype itself, it may just have to >>> check every time it accesses store to get the next piece of the blob - >>> or better performing would be to assume the point is good and have >>> a try/catch to catch the error and turn it into a more reasonable >>> user level error. >>> >>> >>> >>> Sunitha Kambhampati wrote: >>> >>>> I am actually looking at Derby 265 (an assert failure in store). >>>> The assert failure occurs on a getBlob call which is because at >>>> that time there is no transaction context. But then, looking at >>>> the repro got me thinking about select stmt in autocommit mode and >>>> also wonder if the repro is testing the right behavior or not.. >>>> Section 10.1 of the JDBC 3.0 spec says >>>> Enabling autocommit, causes the jdbc driver to do a transaction >>>> commit after each individual sql statement as soon as it is >>>> complete. the point at which it is complete depends on type of >>>> statement. for select statement :- statement is complete when >>>> resultset is closed and result set is closed* as soon as one* of >>>> the following happens >>>> -- all rows have been retrieved >>>> -- associated statement object is re-executed >>>> -- another Statement object is executed on the same connection >>>> >>>> from repro in Derby-265 : >>>> Note s, s2 are on the same connection object that is in autocommit >>>> mode >>>> 1 s.execute("select * from maps") >>>> 2 rs1 = s.getResultSet(); >>>> 3 s2.execute("select * from maps") 4 rs2 = >>>> s2.getResultSet(); 5 rs2.next(); >>>> 6 rs2.getBlob(6); >>>> 7 rs1.close(); >>>> 8 rs2.next(); >>>> 9 rs2.getBlob(6); __________________ >>>> -- from the spec (10.1) , does it mean that the statement execution >>>> on line 3 would commit the earlier statement on #1. ? If so, we >>>> dont seem to do that. >>>> -- Also, rs1.close() is internally calling a commit but the >>>> connection is actually dealing with s2 currently and so is it >>>> right that rs1.close() commits the transaction associated with s2 >>>> ? Then again, is this interleaving of reading of resultsets and >>>> select statement even valid ? . I checked the jdbc spec and the api >>>> and also briefly the tutorial book but didnt come across much about >>>> this. . >>>> >>>> Coming back to the reason for the assert failure >>>> -- so rs1.close() is committing the transaction which is why >>>> rs2.getBlob(6) is left without a transaction context leading to the >>>> assert failure. >>>> A simpler snippet for just the assert failure case (s ,s1 on one >>>> connection in autocommit mode). >>>> 1 s.execute("select * from maps'"); >>>> 2 rs = s.getResultSet(); >>>> 3 s1.executeUpdate("insert .... "); >>>> 4 rs.next(); >>>> 5 rs.getBlob(6); >>>> >>>> -- when s1 is executed , s is complete ( and committed ) per spec. >>>> Will rs still be valid at (line 4), I guess that depends on the >>>> holdability. As rs is a hold cursor, what transaction context >>>> should this be in ? >>>> -- The assert failure happens on the getBlob call ( line 5) , which >>>> is because the blob has an underlying outputstream and uses a >>>> transaction context in this case. >>>> The jdbc api for Blob says ' A blob object_ is valid for the >>>> duration* *of the transaction in which* *it was created_*'*. From >>>> this it seems like the call on #5 is actually not valid ( since >>>> the transaction in which the blob was created is complete). >>>> >>>> -- All this makes me think that the program is incorrect. But I >>>> guess we should be throwing a better user error instead of an >>>> npe/assert. >>>> ___________________ >>>> >>>> Also some notes on derby 265. >>>> -- repro violated this part of the jdbc api for Statement >>>> "By default, only one |ResultSet| object per |Statement| object can >>>> be open at the same time. Therefore, if the reading of one >>>> |ResultSet| object is interleaved with the reading of another, each >>>> must have been generated by *different |Statement| objects*. All >>>> execution methods in the |Statement| interface implicitly close a >>>> statment's current |ResultSet| object if an open one exists" >>>> So made changes to use different Statement objects. >>>> >>>> -- The derby 265 assert failure cause is not specific to network >>>> server mode as such. In the original repro, getBlob() was not >>>> being called in the program which is why embedded was not throwing >>>> the error, but for network server a rs2.next() actually retrieves >>>> the blob (getBlob()) which causes the assert to be thrown at the >>>> store level. So changing the program to call rs2.getBlob shows up >>>> the error in embedded mode also. >>>> >>>> -- Note, the assert failure happens only if the blob column overflows >>>> >>>> I'd appreciate any comments/feedback. >>>> >>>> Thanks, >>>> Sunitha. >>>> >>>> >>>> >>> >>> >> >> >> ------------------------------------------------------------------------ >> >> Index: java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java >> =================================================================== >> --- java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java >> (revision 178571) >> +++ java/engine/org/apache/derby/impl/jdbc/EmbedBlob.java (working >> copy) >> @@ -125,8 +125,16 @@ >> if (SanityManager.DEBUG) >> SanityManager.ASSERT(myStream instanceof Resetable); >> >> - ((Resetable)myStream).initStream(); >> - // set up the buffer for trashing the bytes to set the >> position of the >> + try { >> + ((Resetable) myStream).initStream(); >> + } catch (StandardException se) { >> + if >> (se.getMessageId().equals(SQLState.DATA_CONTAINER_CLOSED)) { >> + throw StandardException >> + >> .newException(SQLState.BLOB_ACCESSED_AFTER_COMMIT); >> + } >> + } >> + // set up the buffer for trashing the bytes to set the >> position of >> + // the >> // stream, only need a buffer when we have a long column >> buf = new byte[BLOB_BUF_SIZE]; >> } >> Index: java/engine/org/apache/derby/impl/jdbc/EmbedClob.java >> =================================================================== >> --- java/engine/org/apache/derby/impl/jdbc/EmbedClob.java >> (revision 178571) >> +++ java/engine/org/apache/derby/impl/jdbc/EmbedClob.java (working >> copy) >> @@ -113,8 +113,14 @@ >> if (SanityManager.DEBUG) >> SanityManager.ASSERT(myStream instanceof Resetable); >> >> - ((Resetable)myStream).initStream(); >> - >> + try { >> + ((Resetable) myStream).initStream(); >> + } catch (StandardException se) { >> + if >> (se.getMessageId().equals(SQLState.DATA_CONTAINER_CLOSED)) { >> + throw StandardException >> + >> .newException(SQLState.BLOB_ACCESSED_AFTER_COMMIT); >> + } >> + } >> } >> } >> >> Index: >> java/engine/org/apache/derby/impl/store/raw/data/OverflowInputStream.java >> >> =================================================================== >> --- >> java/engine/org/apache/derby/impl/store/raw/data/OverflowInputStream.java >> (revision 178571) >> +++ >> java/engine/org/apache/derby/impl/store/raw/data/OverflowInputStream.java >> (working copy) >> @@ -21,6 +21,7 @@ >> package org.apache.derby.impl.store.raw.data; >> >> import org.apache.derby.iapi.error.StandardException; >> +import org.apache.derby.iapi.reference.SQLState; >> >> import org.apache.derby.iapi.store.raw.RecordHandle; >> >> @@ -138,6 +139,12 @@ >> */ >> public void initStream() throws StandardException >> { >> + // it is possible that the transaction in which the stream >> was + // created is committed and no longer valid >> + // dont want to get NPE but instead throw error that >> + // container was not opened >> + if (owner.getTransaction() == null) >> + throw >> StandardException.newException(SQLState.DATA_CONTAINER_CLOSED); >> /* >> We might want to use the mode and isolation level of the >> container. >> This would have the advantage that, if the isolation level >> Index: >> java/engine/org/apache/derby/impl/store/raw/data/BaseContainerHandle.java >> >> =================================================================== >> --- >> java/engine/org/apache/derby/impl/store/raw/data/BaseContainerHandle.java >> (revision 178571) >> +++ >> java/engine/org/apache/derby/impl/store/raw/data/BaseContainerHandle.java >> (working copy) >> @@ -848,12 +848,6 @@ >> */ >> public final RawTransaction getTransaction() { >> - >> - if (SanityManager.DEBUG) - { >> - SanityManager.ASSERT(xact != null); >> - } >> - >> return xact; >> } >> >> Index: >> java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/blobclob4BLOB.java >> >> =================================================================== >> --- >> java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/blobclob4BLOB.java >> >> (revision 178571) >> +++ >> java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/blobclob4BLOB.java >> >> (working copy) >> @@ -167,6 +167,8 @@ >> clobTestSelfDestructive2(conn); >> >> conn.commit(); >> + clobNegativeTest_Derby265(conn); >> + blobNegativeTest_Derby265(conn); >> conn.close(); >> System.out.println("FINISHED TEST blobclob :-)"); >> >> @@ -3785,6 +3787,144 @@ >> } >> } >> >> + + + /** >> + * Test fix for derby-265. >> + * Test that if getBlob is called after the transaction + * >> in which it was created is committed, a proper user error >> + * is thrown instead of an NPE. + * Basically per the spec, >> getBlob is valid only for the duration of + * the transaction in >> it was created in >> + * @param conn >> + * @throws SQLException >> + * @throws FileNotFoundException >> + * @throws IOException >> + */ >> + private static void blobNegativeTest_Derby265(Connection conn) >> + throws SQLException, FileNotFoundException,IOException { >> + // basically setup the tables for clob and blob >> + Statement s = conn.createStatement(); >> + s.execute("create table \"MAPS_BLOB\"(MAP_ID int, MAP_NAME >> varchar(20),REGION varchar(20),AREA varchar(20), PHOTO_FORMAT >> varchar(20),PICTURE blob(2G))"); >> + conn.setAutoCommit(false); >> + PreparedStatement ps = conn.prepareStatement("insert into >> \"MAPS_BLOB\" values(?,?,?,?,?,?)"); >> + + for (int i = 0; i < 3; i++) { >> + FileInputStream fis = new FileInputStream(fileName[4]); >> + ps.setInt(1, i); >> + ps.setString(2, "x" + i); >> + ps.setString(3, "abc"); >> + ps.setString(4, "abc"); >> + ps.setString(5, "abc"); >> + ps.setBinaryStream(6, new >> java.io.BufferedInputStream(fis), 300000); >> + ps.executeUpdate(); >> + fis.close(); >> + } >> + conn.commit(); >> + >> + conn.setAutoCommit(true); >> + System.out.println("-----------------------------"); >> + >> + s = conn.createStatement(ResultSet.TYPE_FORWARD_ONLY, >> + ResultSet.CONCUR_READ_ONLY); >> + s.execute("SELECT \"MAP_ID\", \"MAP_NAME\", \"REGION\", >> \"AREA\", \"PHOTO_FORMAT\", \"PICTURE\" FROM \"MAPS_BLOB\""); >> + ResultSet rs1 = s.getResultSet(); >> + Statement s2 = >> conn.createStatement(ResultSet.TYPE_FORWARD_ONLY, >> + ResultSet.CONCUR_READ_ONLY); >> + s2.executeQuery("SELECT \"MAP_ID\", \"MAP_NAME\", >> \"REGION\", \"AREA\", \"PHOTO_FORMAT\", \"PICTURE\" FROM >> \"MAPS_BLOB\""); >> + ResultSet rs2 = s2.getResultSet(); >> + rs2.next(); >> + >> + Blob b2 = rs2.getBlob(6); >> + rs1.next(); >> + Blob b1 = rs1.getBlob(6); >> + try { >> + rs1.close(); >> + rs2.next(); >> + rs2.getBlob(6); >> + } catch (SQLException sqle) { >> + if ("XJ073".equals(sqle.getSQLState())) >> + System.out.println("Expected Exception " + >> sqle.getMessage()); >> + else >> + System.out.println("FAIL -- unexpected exception:" >> + + sqle.toString()); >> + } >> + finally { >> + rs2.close(); >> + s2.close(); >> + s.close(); >> + ps.close(); >> + } >> + >> + } >> + >> + /** >> + * Test fix for derby-265. >> + * Test that if getClob is called after the transaction + * >> in which it was created is committed, a proper user error >> + * is thrown instead of an NPE. + * Basically per the spec, >> getClob is valid only for the duration of + * the transaction in >> it was created in >> + * @param conn >> + * @throws SQLException >> + * @throws FileNotFoundException >> + * @throws IOException >> + */ >> + private static void clobNegativeTest_Derby265(Connection conn) >> + throws SQLException, FileNotFoundException,IOException { >> + >> + // basically setup the tables for clob + Statement s >> = conn.createStatement(); >> + s.execute("create table \"MAPS\"(MAP_ID int, MAP_NAME >> varchar(20),REGION varchar(20),AREA varchar(20), PHOTO_FORMAT >> varchar(20),PICTURE clob(2G))"); >> + conn.setAutoCommit(false); >> + PreparedStatement ps = conn.prepareStatement("insert into >> \"MAPS\" values(?,?,?,?,?,?)"); >> + for (int i = 0; i < 3; i++) { >> + FileReader fr = new FileReader(fileName[4]); >> + ps.setInt(1, i); >> + ps.setString(2, "x" + i); >> + ps.setString(3, "abc"); >> + ps.setString(4, "abc"); >> + ps.setString(5, "abc"); >> + ps.setCharacterStream(6, new >> java.io.BufferedReader(fr),300000); >> + ps.executeUpdate(); >> + fr.close(); >> + } >> + conn.commit(); >> + >> + conn.setAutoCommit(true); >> + System.out.println("-----------------------------"); >> + s = conn.createStatement(ResultSet.TYPE_FORWARD_ONLY, >> + ResultSet.CONCUR_READ_ONLY); >> + s.execute("SELECT \"MAP_ID\", \"MAP_NAME\", \"REGION\", >> \"AREA\", \"PHOTO_FORMAT\", \"PICTURE\" FROM \"MAPS\""); >> + ResultSet rs1 = s.getResultSet(); >> + Statement s2 = >> conn.createStatement(ResultSet.TYPE_FORWARD_ONLY, >> + ResultSet.CONCUR_READ_ONLY); >> + s2.executeQuery("SELECT \"MAP_ID\", \"MAP_NAME\", >> \"REGION\", \"AREA\", \"PHOTO_FORMAT\", \"PICTURE\" FROM \"MAPS\""); >> + ResultSet rs2 = s2.getResultSet(); >> + rs2.next(); >> + >> + Clob b2 = rs2.getClob(6); // should be fine >> + rs1.next(); >> + Clob b1 = rs1.getClob(6); >> + try { >> + rs1.close(); // this commits the transaction >> + rs2.next(); >> + rs2.getClob(6); // no longer valid >> + } catch (SQLException sqle) { >> + if ("XJ073".equals(sqle.getSQLState())) >> + System.out.println("Expected Exception " + >> sqle.getMessage()); >> + else >> + System.out.println("FAIL -- unexpected exception:" >> + + sqle.toString()); >> + } >> + finally { >> + rs2.close(); >> + s2.close(); >> + s.close(); >> + ps.close(); >> + } >> + >> + } >> static void printInterval(Clob clob, long pos, int length, >> int testNum, int iteration, int clobLength) >> { >> Index: >> java/testing/org/apache/derbyTesting/functionTests/master/DerbyNet/blobclob4BLOB.out >> >> =================================================================== >> --- >> java/testing/org/apache/derbyTesting/functionTests/master/DerbyNet/blobclob4BLOB.out >> >> (revision 178571) >> +++ >> java/testing/org/apache/derbyTesting/functionTests/master/DerbyNet/blobclob4BLOB.out >> >> (working copy) >> @@ -717,5 +717,9 @@ >> Expect to get an IOException, container has been closed >> 10000 total bytes read >> clobTestSelfDestructive2 finished >> +----- >> +Expected Exception The data in this Blob or Clob is no longer >> available. Possible reasons are that its transaction committed, or >> its connection closed. >> +----- >> +Expected Exception The data in this Blob or Clob is no longer >> available. Possible reasons are that its transaction committed, or >> its connection closed. >> FINISHED TEST blobclob :-) >> Test blobclob finished >> Index: >> java/testing/org/apache/derbyTesting/functionTests/master/DerbyNetClient/blobclob4BLOB.out >> >> =================================================================== >> --- >> java/testing/org/apache/derbyTesting/functionTests/master/DerbyNetClient/blobclob4BLOB.out >> >> (revision 178571) >> +++ >> java/testing/org/apache/derbyTesting/functionTests/master/DerbyNetClient/blobclob4BLOB.out >> >> (working copy) >> @@ -717,5 +717,9 @@ >> Expect to get an IOException, container has been closed >> 10000 total bytes read >> clobTestSelfDestructive2 finished >> +----- >> +Expected Exception The data in this Blob or Clob is no longer >> available. Possible reasons are that its transaction committed, or >> its connection closed. >> +----- >> +Expected Exception The data in this Blob or Clob is no longer >> available. Possible reasons are that its transaction committed, or >> its connection closed. >> FINISHED TEST blobclob :-) >> Test blobclob finished >> Index: >> java/testing/org/apache/derbyTesting/functionTests/master/blobclob4BLOB.out >> >> =================================================================== >> --- >> java/testing/org/apache/derbyTesting/functionTests/master/blobclob4BLOB.out >> >> (revision 178571) >> +++ >> java/testing/org/apache/derbyTesting/functionTests/master/blobclob4BLOB.out >> >> (working copy) >> @@ -784,5 +784,9 @@ >> After drop >> Expect to get an IOException, container has been closed >> EXPECTED IO Exception:ERROR 40XD0: Container has been closed >> +----------------------------- >> +Expected Exception The data in this Blob or Clob is no longer >> available. Possible reasons are that its transaction committed, or >> its connection closed. >> +----------------------------- >> +Expected Exception The data in this Blob or Clob is no longer >> available. Possible reasons are that its transaction committed, or >> its connection closed. >> FINISHED TEST blobclob :-) >> Test blobclob finished > > > >
