db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "V.Narayanan (JIRA)" <j...@apache.org>
Subject [jira] Commented: (DERBY-2604) Implement Clob support for locators
Date Wed, 09 May 2007 09:43:15 GMT

    [ https://issues.apache.org/jira/browse/DERBY-2604?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12494462
] 

V.Narayanan commented on DERBY-2604:
------------------------------------

Thanx a ton for looking at this and giving your feedback/comments. Pls find answers to your
queries below. Pls do point out if I have made any mistake in any of the analysis I have done.

>1) readBytes() doesn't look correct to me. It takes a len parameter, 
>which is length in bytes, and passes it on to clobGetSubString(), which 
>expects length in characters. This means that the byte array returned 
>by readBytes() might contain more bytes than len. I cannot see that 
>read() or read(byte[],int,int) are prepared for this.

I agree. I should have taken care of this while submitting the patch.
I intend to allow the code top remain the same way except to change the
code after doing

String resultStr = connection.locatorProcedureCall().
                    clobGetSubString(clob.getLocator(),
                    currentPos, actualLength);

we know that when we do a resultStr.getBytes we would get more
Bytes than actualLength. So from resultStr.getBytes I need to get
only actualLength number of Bytes.

In this conversion from String to Bytes I intend to use the method
getBytesFromString in ClobStreamControl  for the same reasons
the method was used in Derby-2346.

>2) The boundary checking in read(byte[],int,int) might suffer from 
>int overflow. (off+len > b.length) should be replaced with 
>(len > b.length - off).

you are correct. I will do this.

>3) clob and connection could be final, I think.

I will do this.

2) and 3) are valid for ClobReader/Writer also.

>4)I noticed that ClobLocator{Reader,Writer} check whether the object is closed, 
>  whereas ClobLocator{Input,Output}Stream don't. Is there any reason for this?

I read the API doc for java.io.Reader/Writer and saw that it was mentioned that
implementation of the close() method was mandatory here. This is not the case
for InputStream/OutputStream.

Also in the the explanation for the close() method of Reader/Writer it was
mentioned that when a read or write is called after close an IOException is
thrown. But the behaviour that needs to be exhibited for InputStream/OutputStream
methods after close was not mentioned.

>5) ClobLocatorOutputStream.writeBytes() has this line:
>    String clobStr = new String(b);
>   Should it also have specified the encoding?

Yes you are correct. It should have been US-ASCII, I will fix this.

>6) Perhaps it would be easier to make ClobLocatorInputStream a wrapper around 
>ClobLocatorReader, and ClobLocatorOutputStream a wrapper around ClobLocatorWriter?

I thought about this while working on the patch but decided against it because
if you did the wrapping as you suggested. 

The only method that would be shared  would have the functionality of a stored 
procedure call to CLOBGETSTRING to get a string from the Clob. The interconversion 
to Byte array or a Char array will still remain in the respective methods.

There was not much code sharing I could achieve by forwarding calls to the Reader/Writer
implementation and the inter conversion between String and Bytes and Chars still remained.

> Implement Clob support for locators
> -----------------------------------
>
>                 Key: DERBY-2604
>                 URL: https://issues.apache.org/jira/browse/DERBY-2604
>             Project: Derby
>          Issue Type: Sub-task
>          Components: Network Server
>            Reporter: V.Narayanan
>         Assigned To: V.Narayanan
>         Attachments: ClobLocatorWork_v1.diff, ClobLocatorWork_v1.stat
>
>


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