db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Øystein Grøvlen (JIRA) <j...@apache.org>
Subject [jira] Commented: (DERBY-2540) Restructure code for Blob/Clob length in client to prepare for locator implementation
Date Mon, 16 Apr 2007 07:43:15 GMT

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


Mime
View raw message