sheharyaar commented on issue #1212:
URL: https://github.com/apache/kvrocks/issues/1212#issuecomment-2000449365

   @jihuayu  @mapleFU , the PR is almost complete and I have added the test 
cases. However there is a small issue. 
   Currently RawBitPos returns `count_bytes * 8 (i.e. the number of bits)` if 
the test bit is '0' and no such bits are found in the range. It is so that, in 
the function `BitmapStriing::BitPos` we can return -1 if `stop_given` is true 
and `count_bytes * 8` if `stop_given` is false. But if we directly add support 
for bits directly to this function, then in that case it causes a bug.
   
   Example if stored string is "\x00\x00\x00"
   Query > BITPOS mykey 0 2 3 BIT
   
   This should return 2. But if we calculate total count (stop-start+1) that 
comes out to be 2 and also the value returned from BitRawPos is 2 (valid).
   
   Then this relation evalutes to true and returns -1 instead of 2.
   
![image](https://github.com/apache/kvrocks/assets/34273345/4a0d4cf1-7f6e-4113-b439-92ab923f8324)
   
https://github.com/apache/kvrocks/blob/unstable/src/types/redis_bitmap_string.cc#L122-L125
   
   
   *Possible solutions* :
   1. If we fix this in RawBitPos, it will deviate from redis' implementation. 
Redis' RawBitPos returns `bytes*8`, but the bit is handled in the caller 
function (BitmapString::BitPos) in this case.
   2. We handle bits in BitmapString::BitPos and let RawBitPos be the same
   
   This bug doesn't affect Bitmaps as they return -1 even if test bit is 0.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to