[jira] Updated: (DERBY-2496) Implement Blob support for Locators

["Øystein Grøvlen (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/DERBY-2496?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Øystein Grøvlen updated DERBY-2496:
-----------------------------------

    Attachment: blob-followup.diff

Thanks for committing this Rick.  I have responded to your comments
below and the attached patch blob-followup.diff adresses those
comments.  In addition, this patch contains the following changes:

 * My last-minute changes to Lob.sqlLength() introduced a major bug
   that try to materialize the Lob also when locators are used.  (Would
   cause NPE when locators were enabled.)  This has been fixed.

 * Blob.getBinaryStream(long, long) has been implemented.  I had
   missed this since it was added after I started this work.  This
   required changes to BlobLocatorInputStream to handle non-default
   start position and length.

 * The indentation of the new files,
   BlobLocatorInputStream.java/BlobLocatorOutputStream.java was
   irregular in the version that was checked in.  (I do not think it
   was this way in my patch.) I have reindented these files.

> Rick Hillegas commented on DERBY-2496:
> --------------------------------------

> The patch looks good to me. I have a couple small nits, none of
> which would block me from committing this work:
> 
> 1) In the header comment for BlobLocatorOutputStream, I don't
>    understand the reference to ByteArrayInputStream

Sorry, cut&paste error. Have been fixed.

> 
> 2) There are several places in the code where the magic number -1 is
>    used. For instance, the initialization of Lob.locator_. I think
>    it would be more readable and less brittle if this magic number
>    had a friendly name.

I have added a constant, Lob.INVALID_LOCATOR, that is used instead of
the magic number where appropriate.  I have not changed the use of -1
to signal that there is no more data on a stream since the
specification says -1 should be returned, and using a named constant
would just create confusion.

> 
> 3) I wonder if Lob.getLocatorLength() should be abstract and the
>    appropriate subclass could just throw an unimplementedFeature
>    exception right now. It looks to me as though it would be a
>    coding error if, in the released product, a -1 leaked out of this
>    method.

I agree that this should be done, but I would like to wait with this
change since it will probably cause coflicts with Narayanan's work on
Clob. Once, Narayanan has added Clob.getLocatorLength(), I will make
Lob.getLocatorLength abstract.


> Implement Blob support for Locators
> -----------------------------------
>
>                 Key: DERBY-2496
>                 URL: https://issues.apache.org/jira/browse/DERBY-2496
>             Project: Derby
>          Issue Type: Sub-task
>            Reporter: Øystein Grøvlen
>         Assigned To: Øystein Grøvlen
>         Attachments: blob-followup.diff, blob.diff, blob_v2.diff
>
>
> DERBY-2347 adds the possibility to send locators between client and server instead of LOB values.  This has not been activated yet, since the client implementation does not currently support locators.  This report is for supporting the locators for Blob objects.  Another JIRA issue will be made for Clob.
> This work will be made in several steps:
>    1. Blob methods and ResultSet.getXXX methods
>    2. PreparedStatement and CallableStatement methods
>    3. ResultSet.updateXXX methods
>    4. Connection.createBlob()
> There is dependencies between these steps and it might be that the Locator implementation cannot be exposed until everything has been done.  At least, doing just step 1, gives testing errors because tests use Blobs fetched from DB as parameters to prepared statements.   I would guess tests for updatable result sets, needs the combination of 1. and 3.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.