[
https://issues.apache.org/jira/browse/DERBY-2540?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12489031
]
Øystein Grøvlen commented on DERBY-2540:
----------------------------------------
Dag, thanks for the review. I have addressed your comments below, and
I will upload a new patch soon,
> Dag H. Wanvik commented on DERBY-2540:
> --------------------------------------
>
> I think this patch is a good incremental cleanup. I ran derbyall and
> suites.All with (0,2) failures respectively, none related.
> Just some minor nits:
>
> 1) Lob#sqlLength: The method throws SqlException if Layer B streaming
> is used. The javadoc is not clear on this point ("unless Layer B
> streaming is used"). Would be good to move this "unless"-comment to the
> @throws tag.
Good point. Will change the comment.
>
> 2) Lob#materializeStream: Javadoc says "Method to be implemented by
> subclasses to do the necessary setup before calling
> #materializedStream(InputStream, String)". It seems neither
> Blob.java nor Clob.java does any set-up *before* calling
> #materializedStream(InputStream, String). Maybe it would be better
> to say just "Method to be implemented by subclasses, which in
> addition to calling #materializedStream(InputStream, String) will
> do any setup specific to that subclass".
What it actually does it to call it with the right parameters and
assign the result to the right stream. I will update the javadoc to
reflect that.
>
> It also lacks a @throws SqlException tag.
>
Will fix.
>
> 3) Blob#materializeStream: A @throw SqlException is missing
>
Will fix.
> 4) Clob#materializeStream: A @throw SqlException is missing
Will fix.
>
> 5) Clob#length: It seems this method will no longer be checking for
> closed connection; is this correct? Maybe you can
> explain why this change is ok, it seems from the comments this
> is intended.
Clob#length earlier only checked for closed connections if the length
was not already obtained (i.e., the Clob had been materialized). I
could have checked for closed connections in Lob#sqlLength, but the
problem is Blob and Clob currently behave differently. Blob always
checks, Clob only if length needs to be obtained. Hence, if I put the
check in Lob#sqlLength, I would have had to change tests. I wanted to
avoid that since the behavior will soon change again when locators are
introduced. (I guess this means that getting the length of a not
materialized Clob after the connection is closed is not currently
tested.)
>
> 6) BlobOutputStream#writeX: It seems an arraycopy could be used for
> the second part of the copy operation as well (not introduced by
> this patch, though, only refactored, but I thought I'd mention it).
>
I agree that arraycopy seems a better choice, but I do not think I
want to change this code as part of this patch. Also, I do not think
BlobOutputStream will be in much use going forward since new Stream
classes will be made for locator based Blobs.
> Restructure code for Blob/Clob length in client to prepare for locator
> implementation
> -------------------------------------------------------------------------------------
>
> Key: DERBY-2540
> URL: https://issues.apache.org/jira/browse/DERBY-2540
> Project: Derby
> Issue Type: Sub-task
> Components: Network Client
> Reporter: Øystein Grøvlen
> Assigned To: Øystein Grøvlen
> Fix For: 10.3.0.0
>
> Attachments: derby-2540.diff, derby-2540.stat
>
>
> In order to prepare for the locator-based implementation, I want to
> restructure the code for getting the length of LOBs.
> Currently, the LOB class has a protected field sqlLength_ that will contain
> the length of the LOB, if known. Currently, it is always known as long as
> the LOB has been materialized. However, when locators are introduced, the
> length may have to be fetched from the server the first time it is needed.
> With the current implementation, where sqlLength_ is accessed directly by
> many classes in the package, it will become very difficult to keep track of
> whether one needs to fetch the length from the server or not.
> I will change the implementation to make Lob.sqlLength_ private and add
> access methods to get and set its value. (A good thing in itself IMHO). If
> the length is unknown, the LOB will be materialized to get the length.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
