Derby
  1. Derby
  2. DERBY-5090

Retrieving BLOB fields sometimes fails

    Details

    • Urgency:
      Low
    • Issue & fix info:
      Newcomer, Release Note Needed, Repro attached, Workaround attached
    • Bug behavior facts:
      Embedded/Client difference

      Description

      This is my first issue report, so please be understanding if I'm posting the wrong thing, in the wrong place or in the wrong way. I just want to help.

      While iterating through a ResultSet, when accessing a BLOB field to read its contents via an InputStream, I noticed that:

      • if the current ResultSet's has been "warmed up" by retrieving another column first, everything it's fine;
      • if, on the other hand, you first-thing access the BLOB (and read other columns later), then upon reading the first byte out the InputStream bound to the BLOB field (ResultSet.getBinaryStream("col_name")) an IOException is thrown (and IOException's getMessage() method returns null).

      Following is an example, taken from a real application. The two code segments only differ in the fact that a SMALLINT & VARCHAR read is done before/after the BLOB read.

      -Working snippet-
      [...]
      icRelPath[i] = "imm" + File.separator + "ic" + "" + rs.getShort("setIcone") + "" + i + "." + rs.getString("estensione");
      AutoCloseInputStream acis = new AutoCloseInputStream(rs.getBinaryStream("ic" + i));
      if (rs.wasNull())
      icRelPath[i] = null;
      else
      {
      //icRelPath[i] = "imm" + File.separator + "ic" + "" + rs.getShort("setIcone") + "" + i + "." + rs.getString("estensione");
      BufferedInputStream bis = new BufferedInputStream(acis);
      int b = bis.read();//READS FINE
      [...]

      -Broken snippet-
      [...]
      //icRelPath[i] = "imm" + File.separator + "ic" + "" + rs.getShort("setIcone") + "" + i + "." + rs.getString("estensione");
      AutoCloseInputStream acis = new AutoCloseInputStream(rs.getBinaryStream("ic" + i));
      if (rs.wasNull())
      icRelPath[i] = null;
      else
      {
      icRelPath[i] = "imm" + File.separator + "ic" + "" + rs.getShort("setIcone") + "" + i + "." + rs.getString("estensione");
      BufferedInputStream bis = new BufferedInputStream(acis);
      int b = bis.read();//THROWS IOException WITH A null ERROR MESSAGE STRING
      [...]

      1. releaseNote.html
        3 kB
        Kristian Waagan
      2. derby-5090-3a-change_messages.diff
        29 kB
        Kristian Waagan
      3. derby-5090-2a-test.stat
        0.2 kB
        Kristian Waagan
      4. derby-5090-2a-test.diff
        29 kB
        Kristian Waagan
      5. derby-5090-1b-fix.diff
        19 kB
        Kristian Waagan
      6. derby-5090-1a-fix.stat
        0.4 kB
        Kristian Waagan
      7. derby-5090-1a-fix.diff
        19 kB
        Kristian Waagan
      8. Derby5090.java
        0.8 kB
        Knut Anders Hatlen
      9. Derby5090_3.java
        1 kB
        Unai Vivi
      10. Derby5090_2.java
        1.0 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Hide
          Knut Anders Hatlen added a comment -

          Hi Unai,

          Do you have the stack trace for the IOException that is thrown?

          Show
          Knut Anders Hatlen added a comment - Hi Unai, Do you have the stack trace for the IOException that is thrown?
          Hide
          Unai Vivi added a comment -

          Hi Knut,
          thank you for having a look into this.

          Here is the stack trace:

          java.io.IOException
          at org.apache.derby.iapi.services.io.NewByteArrayInputStream.read(Unknown Source)
          at org.apache.commons.io.input.ProxyInputStream.read(ProxyInputStream.java:99)
          at java.io.BufferedInputStream.fill(BufferedInputStream.java:218)
          at java.io.BufferedInputStream.read(BufferedInputStream.java:237)
          at eu.vv_tech.domo.db.Icone.costrIcone(Icone.java:90)
          at eu.vv_tech.domo.db.Icone.<init>(Icone.java:127)
          at org.apache.jsp.insBottone_jsp._jspService(insBottone_jsp.java:188)
          at org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:70)
          at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
          at org.apache.jasper.servlet.JspServletWrapper.service(JspServletWrapper.java:377)
          at org.apache.jasper.servlet.JspServlet.serviceJspFile(JspServlet.java:313)
          at org.apache.jasper.servlet.JspServlet.service(JspServlet.java:260)
          at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
          at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:290)
          at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
          at org.netbeans.modules.web.monitor.server.MonitorFilter.doFilter(MonitorFilter.java:393)
          at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235)
          at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
          at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:233)
          at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:191)
          at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:127)
          at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:102)
          at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:109)
          at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:298)
          at org.apache.coyote.http11.Http11AprProcessor.process(Http11AprProcessor.java:859)
          at org.apache.coyote.http11.Http11AprProtocol$Http11ConnectionHandler.process(Http11AprProtocol.java:579)
          at org.apache.tomcat.util.net.AprEndpoint$Worker.run(AprEndpoint.java:1555)
          at java.lang.Thread.run(Thread.java:619)

          I don't know about the internals of org.apache.derby.iapi.services.io.NewByteArrayInputStream.read(), but my guess is that if you match the Blob with an InputStream and then (before actually using the InputStream) you read other columns, something gets misplaced...

          Show
          Unai Vivi added a comment - Hi Knut, thank you for having a look into this. Here is the stack trace: java.io.IOException at org.apache.derby.iapi.services.io.NewByteArrayInputStream.read(Unknown Source) at org.apache.commons.io.input.ProxyInputStream.read(ProxyInputStream.java:99) at java.io.BufferedInputStream.fill(BufferedInputStream.java:218) at java.io.BufferedInputStream.read(BufferedInputStream.java:237) at eu.vv_tech.domo.db.Icone.costrIcone(Icone.java:90) at eu.vv_tech.domo.db.Icone.<init>(Icone.java:127) at org.apache.jsp.insBottone_jsp._jspService(insBottone_jsp.java:188) at org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:70) at javax.servlet.http.HttpServlet.service(HttpServlet.java:717) at org.apache.jasper.servlet.JspServletWrapper.service(JspServletWrapper.java:377) at org.apache.jasper.servlet.JspServlet.serviceJspFile(JspServlet.java:313) at org.apache.jasper.servlet.JspServlet.service(JspServlet.java:260) at javax.servlet.http.HttpServlet.service(HttpServlet.java:717) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:290) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at org.netbeans.modules.web.monitor.server.MonitorFilter.doFilter(MonitorFilter.java:393) at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:235) at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206) at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:233) at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:191) at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:127) at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:102) at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:109) at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:298) at org.apache.coyote.http11.Http11AprProcessor.process(Http11AprProcessor.java:859) at org.apache.coyote.http11.Http11AprProtocol$Http11ConnectionHandler.process(Http11AprProtocol.java:579) at org.apache.tomcat.util.net.AprEndpoint$Worker.run(AprEndpoint.java:1555) at java.lang.Thread.run(Thread.java:619) I don't know about the internals of org.apache.derby.iapi.services.io.NewByteArrayInputStream.read(), but my guess is that if you match the Blob with an InputStream and then (before actually using the InputStream) you read other columns, something gets misplaced...
          Hide
          Knut Anders Hatlen added a comment -

          Hi Unai,

          JDBC's definition of ResultSet.getBinaryStream() can be found here:
          http://download.oracle.com/javase/6/docs/api/java/sql/ResultSet.html#getBinaryStream(java.lang.String)

          It says:

          ,----

          Note: All the data in the returned stream must be read prior to
          getting the value of any other column. The next call to a getter
          method implicitly closes the stream.
          `----

          So it seems like the specification demands that the stream is closed
          by the calls to getShort() and getString() between the calls to
          getBinaryStream() and read(). And I suppose that the implicit closing
          of the stream is why the IOException is raised by read().

          But although the described behaviour appears to match the
          specification, I think it would be good if the IOException had a
          message text saying the stream was closed, rather than just having a
          null message. I think we should keep this bug report open until we
          have a better error message.

          Show
          Knut Anders Hatlen added a comment - Hi Unai, JDBC's definition of ResultSet.getBinaryStream() can be found here: http://download.oracle.com/javase/6/docs/api/java/sql/ResultSet.html#getBinaryStream(java.lang.String ) It says: ,---- Note: All the data in the returned stream must be read prior to getting the value of any other column. The next call to a getter method implicitly closes the stream. `---- So it seems like the specification demands that the stream is closed by the calls to getShort() and getString() between the calls to getBinaryStream() and read(). And I suppose that the implicit closing of the stream is why the IOException is raised by read(). But although the described behaviour appears to match the specification, I think it would be good if the IOException had a message text saying the stream was closed, rather than just having a null message. I think we should keep this bug report open until we have a better error message.
          Hide
          Knut Anders Hatlen added a comment -

          Attaching a repro for the issue (Derby5090.java).

          With the embedded driver, I get the following exception (no message text):

          Exception in thread "main" java.io.IOException
          at org.apache.derby.iapi.services.io.NewByteArrayInputStream.read(NewByteArrayInputStream.java:53)
          at Derby5090.main(Derby5090.java:22)

          With the client driver, I get this exception (which does say the stream is closed):

          Exception in thread "main" java.io.IOException: The object is already closed.
          at org.apache.derby.client.am.CloseFilterInputStream.read(CloseFilterInputStream.java:50)
          at Derby5090.main(Derby5090.java:22)

          Show
          Knut Anders Hatlen added a comment - Attaching a repro for the issue (Derby5090.java). With the embedded driver, I get the following exception (no message text): Exception in thread "main" java.io.IOException at org.apache.derby.iapi.services.io.NewByteArrayInputStream.read(NewByteArrayInputStream.java:53) at Derby5090.main(Derby5090.java:22) With the client driver, I get this exception (which does say the stream is closed): Exception in thread "main" java.io.IOException: The object is already closed. at org.apache.derby.client.am.CloseFilterInputStream.read(CloseFilterInputStream.java:50) at Derby5090.main(Derby5090.java:22)
          Hide
          Knut Anders Hatlen added a comment -

          However, the call to read() doesn't fail if the Blob is fetched from store and exceeds the size of a page. See the updated repro (Derby5090_2.java). It still raises an IOException with the client driver. The behaviour should be consistent.

          Show
          Knut Anders Hatlen added a comment - However, the call to read() doesn't fail if the Blob is fetched from store and exceeds the size of a page. See the updated repro (Derby5090_2.java). It still raises an IOException with the client driver. The behaviour should be consistent.
          Hide
          Unai Vivi added a comment -

          Oops, how stupid of me to miss that note in the ResultSet's javadoc... I'm glad I put the newcomer "disclaimer"

          Anyways, it's good that this helped to point out that there's this difference between embedded and client drivers, where the former gives a null string message while the latter very helpfully says that the stream is closed.

          The second inconsistency you found is interesting too, where if the Blob is bigger than a page only the client driver throws the object-closed exception while the embedded driver doesn't.

          Show
          Unai Vivi added a comment - Oops, how stupid of me to miss that note in the ResultSet's javadoc... I'm glad I put the newcomer "disclaimer" Anyways, it's good that this helped to point out that there's this difference between embedded and client drivers, where the former gives a null string message while the latter very helpfully says that the stream is closed. The second inconsistency you found is interesting too, where if the Blob is bigger than a page only the client driver throws the object-closed exception while the embedded driver doesn't.
          Hide
          Knut Anders Hatlen added a comment -

          I suppose the stream returned by the embedded driver also will throw an exception on read() for the bigger Blob once all the bytes from the first page have been read, but I haven't checked that. We should at least verify that it throws an exception and doesn't return -1 in that case.

          Show
          Knut Anders Hatlen added a comment - I suppose the stream returned by the embedded driver also will throw an exception on read() for the bigger Blob once all the bytes from the first page have been read, but I haven't checked that. We should at least verify that it throws an exception and doesn't return -1 in that case.
          Hide
          Unai Vivi added a comment -

          Attaching Derby5090_3.java

          Show
          Unai Vivi added a comment - Attaching Derby5090_3.java
          Hide
          Unai Vivi added a comment -

          Hi Knut,
          it actually doesn't. Embedded client returns -1 after all the bytes have been read. [See repro Derby5090_3.java]

          Show
          Unai Vivi added a comment - Hi Knut, it actually doesn't. Embedded client returns -1 after all the bytes have been read. [See repro Derby5090_3.java]
          Hide
          Kristian Waagan added a comment -

          Attaching patches 1a and 2a.
          Patch 1a is the fix, including some clean-up:

          • java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java
            Replace NewByteArrayInputStream with ByteArrayInputStream
            Wrap the above stream in CloseFilterInputStream
          • java/engine/org/apache/derby/iapi/services/io/CloseFilterInputStream.java
            Added filter stream that will throw exception after being closed. Modeled after the equally named class in the client.
          • java/engine/org/apache/derby/iapi/services/io/NewByteArrayInputStream.java
            Deleted this class, use a standard API-class instead.
          • java/engine/org/apache/derby/iapi/services/io/AccessibleByteArrayOutputStream.java
            Replace NewByteArrayInputStream with ByteArrayInputStream
          • java/client/org/apache/derby/client/am/ResultSet.java
            Renamed is_ to currentStream.
            Added instance variable currentReader
            Renamed method closeCloseFilterInputStream to closeOpenStreams
            Modified closeOpenStream to close both currentStream and currentReader (only one will be non-null at any time)
            Assigned reader to currentReader in getCharacterStream

          Patch 2b adds two tests, but I'm a bit unsure if I want to keep JDBCSetGet as a separate class. Will anyone else use it?
          If not, it might be better to keep them as private classes in the test class.
          If we want to add it as a JUnit utility class, should it be split in two, i.e. JDBCGet and JDBCSet? Also, if keeping I'd like to make it possible to obtain the name of the last called getter to improve the error reporting.

          Patches ready for review.

          Show
          Kristian Waagan added a comment - Attaching patches 1a and 2a. Patch 1a is the fix, including some clean-up: java/engine/org/apache/derby/impl/jdbc/EmbedResultSet.java Replace NewByteArrayInputStream with ByteArrayInputStream Wrap the above stream in CloseFilterInputStream java/engine/org/apache/derby/iapi/services/io/CloseFilterInputStream.java Added filter stream that will throw exception after being closed. Modeled after the equally named class in the client. java/engine/org/apache/derby/iapi/services/io/NewByteArrayInputStream.java Deleted this class, use a standard API-class instead. java/engine/org/apache/derby/iapi/services/io/AccessibleByteArrayOutputStream.java Replace NewByteArrayInputStream with ByteArrayInputStream java/client/org/apache/derby/client/am/ResultSet.java Renamed is_ to currentStream. Added instance variable currentReader Renamed method closeCloseFilterInputStream to closeOpenStreams Modified closeOpenStream to close both currentStream and currentReader (only one will be non-null at any time) Assigned reader to currentReader in getCharacterStream Patch 2b adds two tests, but I'm a bit unsure if I want to keep JDBCSetGet as a separate class. Will anyone else use it? If not, it might be better to keep them as private classes in the test class. If we want to add it as a JUnit utility class, should it be split in two, i.e. JDBCGet and JDBCSet? Also, if keeping I'd like to make it possible to obtain the name of the last called getter to improve the error reporting. Patches ready for review.
          Hide
          Knut Anders Hatlen added a comment -

          I had a look at the 1a patch and it looks like a good fix. I was a bit surprised by the use of MessageId.CONN_ALREADY_CLOSED to tell that the stream was closed, but after checking messages.xml, I found that the message text wasn't connection-specific despite the name of the constant.

          One tiny nit in EmbedResultSet:

          • currentStream = stream;
            + // Wrap in a stream throwing exception on invocations when closed.
            + currentStream = stream = new CloseFilterInputStream(stream);
            return stream;

          Perhaps avoiding the double assignment would be slightly clearer:
          currentStream = new CloseFilterInputStream(stream);
          return currentStream;
          ?

          Before the fix, the stream sometimes worked (or perhaps it just appeared to work?) after reading the next column, whereas now it'll always throw an IOException. Should we mention that in the release notes?

          Show
          Knut Anders Hatlen added a comment - I had a look at the 1a patch and it looks like a good fix. I was a bit surprised by the use of MessageId.CONN_ALREADY_CLOSED to tell that the stream was closed, but after checking messages.xml, I found that the message text wasn't connection-specific despite the name of the constant. One tiny nit in EmbedResultSet: currentStream = stream; + // Wrap in a stream throwing exception on invocations when closed. + currentStream = stream = new CloseFilterInputStream(stream); return stream; Perhaps avoiding the double assignment would be slightly clearer: currentStream = new CloseFilterInputStream(stream); return currentStream; ? Before the fix, the stream sometimes worked (or perhaps it just appeared to work?) after reading the next column, whereas now it'll always throw an IOException. Should we mention that in the release notes?
          Hide
          Kristian Waagan added a comment -

          Thanks for the review, Knut.

          I made the double assignment because 'currentStream' is of type Object. I have now split it up into two separate assignments. I'm still returning 'stream' to avoid a cast.

          Attached patch 1b, committed to trunk with revision 1142896.

          Show
          Kristian Waagan added a comment - Thanks for the review, Knut. I made the double assignment because 'currentStream' is of type Object. I have now split it up into two separate assignments. I'm still returning 'stream' to avoid a cast. Attached patch 1b, committed to trunk with revision 1142896.
          Hide
          Kristian Waagan added a comment -

          Attaching patch 3a, which takes a stab at improving the message situation. I found the following relevant messages:
          6 x XCL53 LANG_STREAM_CLOSED Stream is closed
          7 x XJ012.S ALREADY_CLOSED '

          {0}

          ' already closed.
          0 x XJ094.S OBJECT_ALREADY_CLOSED This object is already closed.
          2 x J104 CONN_ALREADY_CLOSED The object is already closed.

          XCL53 doesn't carry an SQLState, so it probably belongs in MessageId. The use of XJ012.S seems OK.
          Patch 3a does the following:
          o remove unused XJ094.S
          o rename J104 to OBJECT_CLOSED.
          o replace uses of XCL53 with J104.
          o remove XCL53

          Alternatively, we could do a swap - use msg/translations for XCL53 in J104 to make it more specific (and rename it to STREAM_CLOSED).

          Patch ready for review.

          Show
          Kristian Waagan added a comment - Attaching patch 3a, which takes a stab at improving the message situation. I found the following relevant messages: 6 x XCL53 LANG_STREAM_CLOSED Stream is closed 7 x XJ012.S ALREADY_CLOSED ' {0} ' already closed. 0 x XJ094.S OBJECT_ALREADY_CLOSED This object is already closed. 2 x J104 CONN_ALREADY_CLOSED The object is already closed. XCL53 doesn't carry an SQLState, so it probably belongs in MessageId. The use of XJ012.S seems OK. Patch 3a does the following: o remove unused XJ094.S o rename J104 to OBJECT_CLOSED. o replace uses of XCL53 with J104. o remove XCL53 Alternatively, we could do a swap - use msg/translations for XCL53 in J104 to make it more specific (and rename it to STREAM_CLOSED). Patch ready for review.
          Hide
          Kristian Waagan added a comment -

          Committed patch 3a to trunk with revision 1155332.

          Attaching the first attempt at a release note.

          Show
          Kristian Waagan added a comment - Committed patch 3a to trunk with revision 1155332. Attaching the first attempt at a release note.
          Hide
          Dag H. Wanvik added a comment -

          Release note looked clear to me. +1

          Show
          Dag H. Wanvik added a comment - Release note looked clear to me. +1
          Hide
          Kristian Waagan added a comment -

          DERBY-4521 has been fixed as part of DERBY-5090. The latter also fixed similar problems with some of the other getXStream methods.

          Show
          Kristian Waagan added a comment - DERBY-4521 has been fixed as part of DERBY-5090 . The latter also fixed similar problems with some of the other getXStream methods.
          Hide
          Kristian Waagan added a comment -

          Since the incorporated issue and this issue have been marked as unsuitable for backport, there is no more work to be done.

          Note that the fix isn't required for proper operation. The fix makes Derby balk if someone tries to do something that's not in accordance with the JDBC specification.

          Show
          Kristian Waagan added a comment - Since the incorporated issue and this issue have been marked as unsuitable for backport, there is no more work to be done. Note that the fix isn't required for proper operation. The fix makes Derby balk if someone tries to do something that's not in accordance with the JDBC specification.
          Hide
          Unai Vivi added a comment -

          Thank you Kristian for the patch and for the release note!
          +1!!

          Show
          Unai Vivi added a comment - Thank you Kristian for the patch and for the release note! +1!!
          Hide
          Knut Anders Hatlen added a comment -

          [bulk update] Close all resolved issues that haven't been updated for more than one year.

          Show
          Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.

            People

            • Assignee:
              Kristian Waagan
              Reporter:
              Unai Vivi
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development