Ooops.

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


We have both 0x8002 and 0x8004 in the patch.
Either of them need comment ....

Best regards.


TomohitoNakayama wrote:

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
       [EMAIL PROTECTED]
       [EMAIL PROTECTED]
       [EMAIL PROTECTED]

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

*/

Reply via email to