[
https://issues.apache.org/jira/browse/DERBY-4023?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681671#action_12681671
]
Knut Anders Hatlen commented on DERBY-4023:
-------------------------------------------
Your comment mentions a 2b patch, but I only see 2a.
Assuming that you meant 2a, the patch basically looks good to me. A couple of
minor comments:
- copyUtf8Data's javadoc doesn't say what it's supposed to return
- "if ((c & 0x80) == 0x00) { // 8th bit set (top bit)" should say "8th bit
*not* set"
- since copyUtf8Data always uses buffered reads, would it make sense not to
wrap a BufferedInputStream around the raw stream in copyClobContent now?
- error messages are not internationalized (should the detailed messages be
asserts instead? the assert in copyClobContent and the EOFException in
copyUtf8Data test the same thing, so perhaps only one is needed?)
- is the chars variable in copyUtf8Data needed, or could charCount be
incremented directly?
- can the stream that is passed into copyUtf8Data ever contain less than 3
bytes? If so, the check for the EOF marker will fail.
- There's the following line in copyUtf8Data: offset = offset % readNow; //
Starting offset for next iteration
Would it be safer if it used minus instead of remainder? I was thinking of
the (perhaps unlikely) case where readNow is 1 and offset>1.
> Improve length caching in TemporaryClob
> ---------------------------------------
>
> Key: DERBY-4023
> URL: https://issues.apache.org/jira/browse/DERBY-4023
> Project: Derby
> Issue Type: Improvement
> Components: JDBC
> Affects Versions: 10.4.2.0, 10.5.0.0
> Reporter: Kristian Waagan
> Assignee: Kristian Waagan
> Priority: Minor
> Fix For: 10.5.0.0
>
> Attachments: derby-4023-1a-cache_length_simple.diff,
> derby-4023-2a-utf8_aware_copy_content.diff
>
>
> TemporaryClob doesn't save the known length of the Clob in all situations.
> The following places in the code should be improved (some easier than others):
> a) TemporaryClob(String,ConChild)
> b) copyClobContent(InternalClob,long) (non-static)
> c) copyClobContent(InternalClob) (non-static)
> There might be additional places to fix too.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
