db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Knut Anders Hatlen (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-4023) Improve length caching in TemporaryClob
Date Fri, 13 Mar 2009 10:41:51 GMT

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


Mime
View raw message