Knut Anders Hatlen wrote On 02/15/06 16:23,:
"V.Narayanan (JIRA)" <[email protected]> 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
