db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Matrigali <mikem_...@sbcglobal.net>
Subject Re: [PATCH] Derby-265 -In Network Server retrieving BLOB values with autocommit off causes NullPointerException in INSANE build /Assert failure in sane build
Date Fri, 27 May 2005 17:28:24 GMT
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


Mime
View raw message