Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: <derby-dev@db.apache.org>
Message-ID: <28325320.1176709395523.JavaMail.jira@brutus>
Date: Mon, 16 Apr 2007 00:43:15 -0700 (PDT)
From: =?utf-8?Q?=C3=98ystein_Gr=C3=B8vlen_=28JIRA=29?= <jira@apache.org>
To: derby-dev@db.apache.org
Subject: [jira] Commented: (DERBY-2540) Restructure code for Blob/Clob
 length in client to prepare for locator implementation
In-Reply-To: <12494025.1176293912284.JavaMail.jira@brutus>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable


    [ https://issues.apache.org/jira/browse/DERBY-2540?page=3Dcom.atlassian=
.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12489031 ]=20

=C3=98ystein Gr=C3=B8vlen 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:
> --------------------------------------
>=20
> I think this patch is a good incremental cleanup. I ran derbyall and
> suites.All with (0,2) failures respectively, none related.=20
> Just some minor nits:
>=20
> 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 th=
e
>    @throws tag.

Good point.  Will change the comment.

>=20
> 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.

>=20
>    It also lacks a @throws SqlException tag.
>

Will fix.

>=20
> 3) Blob#materializeStream: A @throw SqlException is missing
>=20

Will fix.

> 4) Clob#materializeStream: A @throw SqlException is missing

Will fix.

>=20
> 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.)

>=20
> 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).
>=20

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 im=
plementation
> -------------------------------------------------------------------------=
------------
>
>                 Key: DERBY-2540
>                 URL: https://issues.apache.org/jira/browse/DERBY-2540
>             Project: Derby
>          Issue Type: Sub-task
>          Components: Network Client
>            Reporter: =C3=98ystein Gr=C3=B8vlen
>         Assigned To: =C3=98ystein Gr=C3=B8vlen
>             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 restr=
ucture the code for getting the length of LOBs.=20
> Currently, the LOB class has a protected field sqlLength_ that will conta=
in 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, t=
he length may have to be fetched from the server the first time it is neede=
d.   With the current implementation, where sqlLength_ is accessed directly=
 by many classes in the package, it will become very difficult to keep trac=
k 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 a=
ccess 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.=20

--=20
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.