Derby
  1. Derby
  2. DERBY-2992

getBinaryStream returns incorrect result (truncated value) if underlying blob is deleted

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.2.2.0, 10.3.1.4, 10.4.1.3, 10.5.3.0, 10.6.1.0
    • Fix Version/s: 10.5.3.1, 10.6.1.0
    • Component/s: JDBC
    • Labels:
      None
    • Urgency:
      Normal
    • Issue & fix info:
      High Value Fix, Repro attached
    • Bug behavior facts:
      Regression, Wrong query result

      Description

      If getBinaryStream is reading a value (READ_UNCOMMITTED) and the row is deleted by another connection, a truncated value will be returned without error. I believe instead either the whole value or an IOException should occur.

      With 10.2 and higher with the repro attahed we get:
      > java TruncatedBlob
      Embedded:
      Read 32669 bytes
      0 rows in BLOBCLOB

      With 10.1
      Embedded:
      Read 40000 bytes (OK)
      0 rows in BLOBCLOB

      Note network server returns the full value for both 10.1 and 10.2 but gives a lock timeout for 10.2+. I will file a separate issue for that.

      1. derby-2992-1b-throw_exception_on_eof.diff
        6 kB
        Kristian Waagan
      2. derby-2992-1a-throw_exception_on_eof.diff
        6 kB
        Kristian Waagan
      3. TruncatedBlob.java
        2 kB
        Kathey Marsden

        Issue Links

          Activity

          Hide
          Rick Hillegas added a comment -

          Triaged for 10.5.2: Assigned normal urgency and noted that repro is available.

          Show
          Rick Hillegas added a comment - Triaged for 10.5.2: Assigned normal urgency and noted that repro is available.
          Hide
          Kristian Waagan added a comment -

          From a small experiment running the attached repro, where I made OverflowInputStream throw an exception if it detects that the specified overflow page cannot be found, the following lock dumps shows that an extra lock is set when using the client driver:

            • Embedded:
              /---- Lock dump 1
              477 | TABLE | IS | BLOBTAB | Tablelock | GRANT | T | 1 | null |
              Lock dump 1 -----/
              Caught IOException:EOF reached prematurely
              0 rows in BLOBCLOB
            • Client:
              /---- Lock dump 1
              368 | TABLE | IS | BLOBTAB | Tablelock | GRANT | T | 2 | null |
              368 | ROW | S | BLOBTAB | (2,6) | GRANT | T | 1 | null |
              Lock dump 1 -----/
              // Results in lock timeout.

          Haven't investigated further, but the repro code is identical except for the connection URL so there is definitely a difference between embedded and client/server.
          Also, are there any valid situations where OverflowInputStream.fillByteHolder should fail to get the overflow page when an overflow page has been specified?
          (the next overflow page is set by a callback, according to the comments)

          Finally, if the suggested fix is pursued, we might want to beef up the error message. Any suggestions?

          Show
          Kristian Waagan added a comment - From a small experiment running the attached repro, where I made OverflowInputStream throw an exception if it detects that the specified overflow page cannot be found, the following lock dumps shows that an extra lock is set when using the client driver: Embedded: /---- Lock dump 1 477 | TABLE | IS | BLOBTAB | Tablelock | GRANT | T | 1 | null | Lock dump 1 -----/ Caught IOException:EOF reached prematurely 0 rows in BLOBCLOB Client: /---- Lock dump 1 368 | TABLE | IS | BLOBTAB | Tablelock | GRANT | T | 2 | null | 368 | ROW | S | BLOBTAB | (2,6) | GRANT | T | 1 | null | Lock dump 1 -----/ // Results in lock timeout. Haven't investigated further, but the repro code is identical except for the connection URL so there is definitely a difference between embedded and client/server. Also, are there any valid situations where OverflowInputStream.fillByteHolder should fail to get the overflow page when an overflow page has been specified? (the next overflow page is set by a callback, according to the comments) Finally, if the suggested fix is pursued, we might want to beef up the error message. Any suggestions?
          Hide
          Kristian Waagan added a comment -

          Attached patch 1a, which fixes the problem as outlined in the comment above. I also incorporated the repro/test Kathey has written.
          Should we try to say something about what went wrong in the error message, such that the user has a chance to understand what went wrong?

          Patch ready for review.

          Show
          Kristian Waagan added a comment - Attached patch 1a, which fixes the problem as outlined in the comment above. I also incorporated the repro/test Kathey has written. Should we try to say something about what went wrong in the error message, such that the user has a chance to understand what went wrong? Patch ready for review.
          Hide
          Kristian Waagan added a comment -

          Regression tests passed.

          Show
          Kristian Waagan added a comment - Regression tests passed.
          Hide
          Knut Anders Hatlen added a comment -

          The approach in the patch looks fine to me. I think you're right that the error message is somewhat too developer-centric, and it may be difficult for users to understand what the problem is. What about this instead: "Could not read further because another transaction has modified the value." ?

          Show
          Knut Anders Hatlen added a comment - The approach in the patch looks fine to me. I think you're right that the error message is somewhat too developer-centric, and it may be difficult for users to understand what the problem is. What about this instead: "Could not read further because another transaction has modified the value." ?
          Hide
          Kristian Waagan added a comment -

          Thanks for looking at the patch, Knut Anders.

          Your proposed message sound better to me, and I think it doesn't lie That is, I believe there aren't any other valid reasons causing that exception to be thrown. If you use a stricter isolation level I think locks will be put in place to avoid that a different transaction deletes the value you are currently accessing.
          The only other reason I can think of, is if we have a bug in the store specifying the wrong overflow page id.

          I'll change the error message and commit the patch shortly.

          Show
          Kristian Waagan added a comment - Thanks for looking at the patch, Knut Anders. Your proposed message sound better to me, and I think it doesn't lie That is, I believe there aren't any other valid reasons causing that exception to be thrown. If you use a stricter isolation level I think locks will be put in place to avoid that a different transaction deletes the value you are currently accessing. The only other reason I can think of, is if we have a bug in the store specifying the wrong overflow page id. I'll change the error message and commit the patch shortly.
          Hide
          Kristian Waagan added a comment -

          Attached patch 1b and committed it to trunk with revision 908586.

          Show
          Kristian Waagan added a comment - Attached patch 1b and committed it to trunk with revision 908586.
          Hide
          Kristian Waagan added a comment -

          Back-porting this fix requires some manual work, due to conflicts.
          Does anyone feel strong about back-porting the fix?

          Show
          Kristian Waagan added a comment - Back-porting this fix requires some manual work, due to conflicts. Does anyone feel strong about back-porting the fix?
          Hide
          Kristian Waagan added a comment -

          I haven't received any feedback on the back-porting issue.
          Resolving issue, ready for verification.

          Show
          Kristian Waagan added a comment - I haven't received any feedback on the back-porting issue. Resolving issue, ready for verification.
          Hide
          Mike Matrigali added a comment -

          I am looking at backporting this to the 10.5 branch.

          Show
          Mike Matrigali added a comment - I am looking at backporting this to the 10.5 branch.
          Hide
          Mike Matrigali added a comment -

          backporting change #908586 from trunk to 10.5 branch.

          m105_jdk16:178>svn commit
          Sending .
          Sending java/engine/org/apache/derby/impl/store/raw/data/OverflowInputStream.java
          Sending java/engine/org/apache/derby/loc/messages.xml
          Sending java/shared/org/apache/derby/shared/common/reference/MessageId.java
          Sending java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/BLOBTest.java
          Transmitting file data ....

          Show
          Mike Matrigali added a comment - backporting change #908586 from trunk to 10.5 branch. m105_jdk16:178>svn commit Sending . Sending java/engine/org/apache/derby/impl/store/raw/data/OverflowInputStream.java Sending java/engine/org/apache/derby/loc/messages.xml Sending java/shared/org/apache/derby/shared/common/reference/MessageId.java Sending java/testing/org/apache/derbyTesting/functionTests/tests/jdbcapi/BLOBTest.java Transmitting file data ....
          Hide
          Mike Matrigali added a comment -

          backported to 10.5. resolving as fixed and resetting original owner.

          Show
          Mike Matrigali added a comment - backported to 10.5. resolving as fixed and resetting original owner.

            People

            • Assignee:
              Kristian Waagan
              Reporter:
              Kathey Marsden
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development