[
https://issues.apache.org/jira/browse/DERBY-2247?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12472739
]
Knut Anders Hatlen commented on DERBY-2247:
-------------------------------------------
Some post-commit questions/suggestions:
1) Wouldn't it be simpler to write
BaseStorageFactory.createTemporaryFile() in terms of
File.createTempFile(String prefix, String suffix, File directory)?
2) EmbedBlob.setBinaryStream() has one check for (pos - 1 > length)
and one check for (pos > length()). Should the second test also use
pos-1 and/or could they be written as one test?
3) The second use of BLOB_POSITION_TOO_LARGE in
EmbedBlob.setBinaryStream() lacks the position argument.
4) EmbedBlob.truncate() has this code:
+ if (len > length())
+ throw Util.generateCsSQLException(
+ SQLState.BLOB_NONPOSITIVE_LENGTH, new Long(pos));
Isn't the SQL state wrong here?
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).
6) Couldn't LOBInputStream.read(byte[]) be implemented as
public int read(byte[] b) throws IOException {
return read(b, 0, b.length);
}
?
7) Same comment for LOBOutputStream.write(byte[] b). Could be
implemented as a call to write(b, 0, b.length).
8) All the methods of LOBInputStream and LOBOutputStream start with
+ if (closed)
+ throw new IOException (
+ MessageService.getTextMessage(
+ SQLState.LANG_STREAM_CLOSED));
Perhaps the code would be a bit clearer and easier to keep consistent
if these checks were moved into a utility method?
9) The patch introduced a new SQL state LANG_STREAM_CLOSED =
"42Z12". Since it starts with 42, the generated exceptions will be
of type SQLSyntaxErrorException, which according to the javadoc
indicate "that the in-progress query has violated SQL syntax
rules." Does this description match the situations where it is
used?
10) EmbedBlob's constructor has this code:
+ catch (IOException e) {
+ throw StandardException.newException (null, e);
+ }
I'm a bit worried that passing null as message id could cause
problems. I think we will get a NullPointerException when
StandardException's constructor calls getSeverityFromIdentifier(null).
11) LOBStreamControl.init() might ignore some exceptions.
12) Is this calculation in LOBStreamControl.write() correct?
+ long finalLen = (dataBytes != null) ? dataBytes.length + b.length
+ : b.length;
If (pos!=dataBytes.length) and/or (len!=b.length) finalLen will be too
large, won't it?
13) LOBStreamControl.write: Do we need the if? I guess seek() would be
a no-op anyway if getFilePointer() == pos.
+ if (tmpFile.getFilePointer() != pos)
+ tmpFile.seek(pos);
> 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-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.
