db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From TomohitoNakayama <tomon...@basil.ocn.ne.jp>
Subject Re: [jira] Commented: (DERBY-1471) Implement layer B streaming for new methods defined in JDBC4.0
Date Thu, 07 Dec 2006 11:34:25 GMT
Thank you for your attention.

I think another patch is needed before committing ...

>* BrokeredPreparedStatement now implements length-less setBinaryStream
>  and setCharacterStream. We could therefore remove them from
>  BrokeredPreparedStatement40.
>
>* In DRDAConnThread.readAndSetExtParam(), some of the code has been
>  changed from
>
>    if (stream == null) {
>       ...
>    } else {
>       ...
>    }
>
>  to
>
>    if (stream == null) {
>        ...
>    } else if (stream instanceof StandardEXTDTAReaderInputStream) {
>        ... // basically do the same as in the old else clause
>    } else if (stream instanceof LayerBStreamedEXTDTAReaderInputStream) {
>        ...
>    }
>
>  I would feel more comfortable if the code in the first else-if
>  clause were put into an else clause. If the stream is of another
>  type (shouldn't happen, but someone might extend the code later), it
>  will be silently ignored.
>
I see.
I will fix those in next patch.


>* In NetStatementRequest, 0x8002 is used as a magic number many
>  places. Perhaps it could be explained in a comment, or a constant
>  could be used instead?
>
This number comes from specification of DRDA.
It told to set the Layer B 2-byte object length field to X'8004'.

I think comment is needed in the code.


>* Inner class PublicBufferOutputStream in DRDAConnThread could be
>  private. Or even better, since ReEncodedInputStream also implements
>  such a class, it could be implemented as a stand-alone class that
>  could be shared between DRDAConnThread and ReEncodedInputStream.
>
I think PublicBufferOutputStream should not be shared between classes,
because accessing internal buffer is a kind of a necessary evil and
should not be used widely ...

Well ... I will make it as private.


>* Blob/Clob: The variable willBeLayerBStreamed_ and the method
>  willBeLayerBStreamed() could be moved to the base class (Lob) to
>  avoid duplicated code.
>
I am somewhat repulsed for sharing code around instance variable ....

However, the value never be changed in the life of the object and
they are equal in semantics ....

I will try to modify the code and will judge the answer reading the result.


Best regards.


Knut Anders Hatlen (JIRA) wrote:

>    [ http://issues.apache.org/jira/browse/DERBY-1471?page=comments#action_12456093 ]

>            
>Knut Anders Hatlen commented on DERBY-1471:
>-------------------------------------------
>
>I have looked at patch 3. I don't know enough about layer B streaming
>to say whether all details of the implementation are correct, but I
>have a couple of comments. All of the comments are about minor issues,
>so it would be perfectly OK to commit the patch as it is and address
>the comments in a followup patch.
>
>* BrokeredPreparedStatement now implements length-less setBinaryStream
>  and setCharacterStream. We could therefore remove them from
>  BrokeredPreparedStatement40.
>
>* In DRDAConnThread.readAndSetExtParam(), some of the code has been
>  changed from
>
>    if (stream == null) {
>       ...
>    } else {
>       ...
>    }
>
>  to
>
>    if (stream == null) {
>        ...
>    } else if (stream instanceof StandardEXTDTAReaderInputStream) {
>        ... // basically do the same as in the old else clause
>    } else if (stream instanceof LayerBStreamedEXTDTAReaderInputStream) {
>        ...
>    }
>
>  I would feel more comfortable if the code in the first else-if
>  clause were put into an else clause. If the stream is of another
>  type (shouldn't happen, but someone might extend the code later), it
>  will be silently ignored.
>
>* Inner class PublicBufferOutputStream in DRDAConnThread could be
>  private. Or even better, since ReEncodedInputStream also implements
>  such a class, it could be implemented as a stand-alone class that
>  could be shared between DRDAConnThread and ReEncodedInputStream.
>
>* In NetStatementRequest, 0x8002 is used as a magic number many
>  places. Perhaps it could be explained in a comment, or a constant
>  could be used instead?
>
>* Blob/Clob: The variable willBeLayerBStreamed_ and the method
>  willBeLayerBStreamed() could be moved to the base class (Lob) to
>  avoid duplicated code.
>
>  
>
>>Implement layer B streaming for new methods defined in JDBC4.0
>>--------------------------------------------------------------
>>
>>                Key: DERBY-1471
>>                URL: http://issues.apache.org/jira/browse/DERBY-1471
>>            Project: Derby
>>         Issue Type: New Feature
>>         Components: Network Client
>>           Reporter: Tomohito Nakayama
>>        Assigned To: Tomohito Nakayama
>>        Attachments: DERBY-1471.diff, DERBY-1471.patch, DERBY-1471.stat, DERBY-1471_2.patch,
DERBY-1471_2.stat, DERBY-1471_3.patch, DERBY-1471_3.stat
>>
>>
>>JDBC 4.0 introduced new methods which take parameters for object to be sent to sever
without length information.
>>For those methods, Layer B streaming is best way to implement sending object to server.
>>This issue is representation of DERBY-1417 in Network Client.
>>    
>>
>
>  
>

-- 
/*

        Tomohito Nakayama
        tomonaka@basil.ocn.ne.jp
        tomohito@rose.zero.ad.jp
        tmnk@apache.org

        Naka
        http://www5.ocn.ne.jp/~tomohito/TopPage.html

*/ 


Mime
View raw message