[jira] Updated: (DERBY-2247) provide set methods for blob in embeded driver

["Anurag Shekhar (JIRA)" <ji...@apache.org>]

     [ 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-followup2.diff

>1) Wouldn't it be simpler to write
>   BaseStorageFactory.createTemporaryFile() in terms of
>   File.createTempFile(String prefix, String suffix, File directory)? 

I thought it may be useful in case some other module decides to use this same service 
and want to generate file name based of some pattern. 

>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

>11) LOBStreamControl.init() might ignore some exceptions. 

Added all skipped exception in unexpected exception.


>1) Can control be final?

ClobStreamControl extends from LOBStreamControl. ClobStreamControl is already final.

>2) Comment why AIOOB is thrown instead of IOException?

in case of BLOB_INVALID_OFFSET streams are expected to throw AIOOB.


> 7) In write(byte[],int,int,long), all SQLExceptions except one (BLOB_INVALID_OFFSET) are silently ignored. >Is this intended?

fixed


> 8) In free(), is there a chance we ignore some exceptions? If not, add a comment? Or, if it is possible, >create a RuntimeException from the PAE and throw it as last resort?

It won't be good idea to ignore exception from free as most of the time it will be a i/o error. If we throw a non runtime exception it will be trapped and recorded somewhere in the users of these methods hence making it easy to report problem.
added a new throw statement to wrap all no IOException uder IOException.


[BlobSetMethodsTest]
> 9) Comment why the test is currently only run in embedded in suite().

enabled it for client too
 
>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.

fixed

>11) In tearDown(), you could use "Statement stmt = createStatement()" and at last call super.tearDown().

added
[LobStreamTest]

>12) JavaDoc for testReadWriteNoParameters (and the method name) is a little off.
fixed

>13) For the assertEquals comparing input and output, I think it should say "Output does not match input" or >something similar.
fixed
>14) Replace assertTrue(msg, false) with fail(msg).
fixed

>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.



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 <?

changed

>Actually, I'm also wondering whether this test in LOBStreamControl.write(int,long)
>           if (pos + 1 < MAX_BUF_SIZE) {
>should have been (pos < MAX_BUF_SIZE).

>And should this test in LOBStreamControl.truncate()
            if (size < Integer.MAX_VALUE && size < MAX_BUF_SIZE) {
>have been (size <= MAX_BUF_SIZE)? 

fixed


> 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-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.