db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anurag Shekhar (JIRA)" <j...@apache.org>
Subject [jira] Updated: (DERBY-2247) provide set methods for blob in embeded driver
Date Wed, 23 May 2007 06:14:16 GMT

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

Anurag Shekhar updated DERBY-2247:

    Attachment: derby-2247-followupv3.diff

>No, I didn't notice the difference in signatures. It should still
>return StorageFile, so the method would look something like this:

>    public StorageFile createTemporaryFile(String prefix, String suffix)
        throws IOException
>    {
>        return newStorageFile(File.createTempFile(
>                prefix, suffix, new File(getTempDir().getPath())).getPath());
>    }

updated with a minor change passing directory in newStorageFile (without this its tries to
make absulute path starting from db direcotry)

>>5) LOBInputStream.read(byte[]) and read(byte[],int,int) contain this
>> piece of code:
>>+ int ret = control.read(b, off, len, pos);
>>+ if (ret > 0) {
>>+ pos += ret;
>>+ return ret;
>>+ }
>>+ return -1;
>>Since a call to InputStream.read(byte[]...) theoretically can return
>>0, I think (ret > 0) should be changed to (ret >= 0) or perhaps (ret
>>!= -1).
>this was taken care by one of the privious patch

>The previous patch removed read(byte[]), but read(byte[],int,int) is
>still there with the same code.


>>11) LOBStreamControl.init() might ignore some exceptions.
>Added all skipped exception in unexpected exception.

>I'm not sure unexpectedUserException() is the correct method to use to
>wrap the exception in this case. I thought it was supposed to be used
>to wrap exceptions thrown by user code (like stored procedures and
>VTIs). Perhaps Util.javaException() would be better?

It was wrong. I am now throw IOException (assuming most of the time it will be a i/o error
apart form RuntimeException and StandardException) with cause set to original exception.

>>1) Can control be final?
>ClobStreamControl extends from LOBStreamControl. ClobStreamControl is
>already final.

>I think Kristian was referring to the variable control in
>LOBOutputStream. It is not final.

changed control in LOBInputStrea and LOBOutputStream to final

>>10) I think it would be good to split the testSetBytes into one test
>>method for in-memory operation, and one for on disk operation.

>It would also be good if these test cases called close() on
>rs. Another thing I noticed is that the test cases contain clean-up
>code in a finally block. If an exception is thrown in the finally
>block, it will hide the original error, so I think it's better to
>remove the finally block.

added close and removed fianlly block

>>15) Comment why the test is currently only run in embedded in suite().
>these tests, the way they are written, are specific to embedded
>(testing with memory and then file system) and the client already has
>similar tests in jdbcapi/LobStreamsTest.

>Could you add this comment in the source file as well? Would it run in
>client/server mode? If so, I think it would be valuable to run it
>under the client too.

updated comment in suite method

Some other comments to LobStreamTest:

  - tearDown() doesn't call super.tearDown() (if it did, calling
    rollback() and close() on conn wouldn't be necessary)

  - the instance variables dbName, useLOBStreamControl and f are not
    used and should be removed

  - the instance variable conn is not needed since the connection is
    also stored in the super class

  - blob should be set to null in tearDown()

called super.tearDown and set blob to null and removed rollback and close call.

>Follow up comments from knuth
>>- long finalLen = (dataBytes != null) ? dataBytes.length + b.length
>>- : b.length;
>>- if (finalLen < MAX_BUF_SIZE)
>>+ if (pos + b.length < MAX_BUF_SIZE)
>> return updateData(b, off, len, pos);
>> else {
>> init(dataBytes, pos);
>> }
>>Shouldn't b.length have been len, in case we don't write the entire byte array?
>>And shouldn't the comparison use <= instead of <?

I noticed that you didn't change the comparison operator. Do you think
that comment was wrong?

i had missed it changed it in the current patch.

> provide set methods for blob in embeded driver
> ----------------------------------------------
>                 Key: DERBY-2247
>                 URL: https://issues.apache.org/jira/browse/DERBY-2247
>             Project: Derby
>          Issue Type: Sub-task
>          Components: JDBC
>         Environment: all
>            Reporter: Anurag Shekhar
>         Assigned To: Anurag Shekhar
>            Priority: Minor
>         Attachments: derby-2247-followup.diff, derby-2247-followup2.diff, derby-2247-followup2.diff,
derby-2247-followupv3.diff, derby-2247-v3-usingStoreFactory.diff, derby-2247-v4-usingStoreFactory.diff,
derby-2247.diff, derby-2247v2-using_StoreFactory.diff

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

View raw message