db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "V.Narayanan" <V.Naraya...@Sun.COM>
Subject Re: [jira] Commented: (DERBY-796) jdbc 4.0 specific Blob and Clob method support
Date Wed, 15 Feb 2006 11:15:37 GMT


Knut Anders Hatlen wrote On 02/15/06 16:23,:

>"V.Narayanan (JIRA)" <derby-dev@db.apache.org> writes:
>
>  
>
>>V.Narayanan commented on DERBY-796:
>>-----------------------------------
>>    
>>
>
>  
>
>>Issue 5
>>-------------
>>
>>--- java/client/org/apache/derby/client/am/Blob.java    (revision 377622)
>>+++ java/client/org/apache/derby/client/am/Blob.java    (working copy)
>>@@ -267,7 +267,10 @@
>>
>>      public int setBytesX(long pos, byte[] bytes, int offset, int len) throws SqlException
{
>>          int length = 0;
>>-        if ((int) pos <= 0) {
>>+        boolean insertIntoEmpty = false;
>>+    if(pos==1 && binaryString_.length==0)
>>+        insertIntoEmpty = true;
>>+        if ((int) pos <= 0 || ((!insertIntoEmpty) && (pos > binaryString_.length
- dataOffset_))) {
>>              throw new SqlException(agent_.logWriter_,
>>                  new MessageId(SQLState.BLOB_BAD_POSITION), new Long(pos));
>>          }
>>
>>It seemed like the essence of this change was something like:
>>
>>   "It's usually an error to pass a 'pos' value greater than the
>>   length of the blob, but if the blob is currently empty then
>>   there is a special case where the caller passes 1, not 0, as
>>   you might expect." 
>>
>>The interpretation above is exactly what I tried to do. I will try
>>to explain what I intended and also restructure the code to make it
>>more understandable.
>>
>>There are two cases when exception needs to be thrown
>>
>>a) when pos <=0
>>b) when pos > binaryString_.length - dataOffset_
>>   b.1) This case arises when your insert into a empty Blob. so here
>>        pos = 1 and (binaryString_.length - dataOffset_)=0.
>>        this should not result in a SQL exception being thrown.
>>    
>>
>
>Is the empty blob really a special case? I think you get the same
>problem when the blob is not empty.
>
>Let's say you want to append a byte array to a blob with size 1. Then
>you have:
>
>  pos = 2
>  (binaryString_.length - dataOffset_) = 1
>
>In this case, (pos > binaryString_ - dataOffset_) is true and an
>exception is thrown. But there's no reason to throw an exception in
>this case, is it?
>
>I think you have to change this code:
>
>  public int setBytesX(long pos, byte[] bytes, int offset, int len) throws SqlException
{
>      int length = 0;
>      boolean insertIntoEmpty = false;
>      if(pos==1 && binaryString_.length==0)
>          insertIntoEmpty = true;
>      if ((int) pos <= 0 || ((!insertIntoEmpty) && (pos > binaryString_.length
- dataOffset_))) {
>          throw new SqlException(agent_.logWriter_,
>              new MessageId(SQLState.BLOB_BAD_POSITION), new Long(pos));
>      }
>      if ( pos > binaryString_.length - dataOffset_) {
>          throw new SqlException(agent_.logWriter_, 
>              new MessageId(SQLState.BLOB_POSITION_TOO_LARGE), new Long(pos));
>      }
>
>to
>
>  public int setBytesX(long pos, byte[] bytes, int offset, int len) throws SqlException
{
>      int length = 0;
>      if ((int) pos <= 0) {
>          throw new SqlException(agent_.logWriter_,
>              new MessageId(SQLState.BLOB_BAD_POSITION), new Long(pos));
>      }
>      if (pos - 1 > binaryString_.length - dataOffset_) {
>          throw new SqlException(agent_.logWriter_, 
>              new MessageId(SQLState.BLOB_POSITION_TOO_LARGE), new Long(pos));
>      }
>
>This will solve both the special case of inserting into an empty Blob,
>and the more general case of inserting at the end.
>  
>
I agree. This will be a more generalized approach. Thank you for this 
one!! I will do this.

>>Issue - 6
>>-----------------
>>
>>it is a cleaner approach to do the conversion from one-based index
>>to zero-based index on the highest possible level
>>-------------------------------------------------
>>
>>Would it be acceptable if I add a proper comment explaining my
>>change for now and raise this as a seperate issue and fix it at the
>>earliest?
>>    
>>
>
>It seems like the Blob/Clob code has a lot of potential
>off-by-ones. Since this is somewhat outside the scope of your patch, I
>think it is OK that you just include the simple (offset_ - 1) fixes
>for now (they seem to be sufficient to fix the bugs you have
>encountered), and address the cleanup of the offset/position confusion
>in a separate JIRA issue.
>  
>

Thanks once again
Narayanan

Mime
View raw message