fix container iteration bugs in roaring

This commit is contained in:
Shaquille Wyan Que 2019-06-20 20:50:58 -05:00
parent fe83ef59c7
commit a6ba7e339c
3 changed files with 223 additions and 24 deletions

View file

@ -452,6 +452,16 @@ func (b *Bitmap) CountRange(start, end uint64) (n uint64) {
// TODO remove once we've validated this stuff works
panic("should be impossible for k to be less than skey")
}
if roaringParanoia {
if start > end {
panic(fmt.Sprintf("counting in range but %v > %v", start, end))
}
}
// k > ekey handles the case when start > end and where start and end
// are in different containers. Same container case is already handled above.
if k > ekey {
break
}
if k == skey {
n += uint64(c.countRange(int32(lowbits(start)), maxContainerVal+1))
continue
@ -464,9 +474,6 @@ func (b *Bitmap) CountRange(start, end uint64) (n uint64) {
n += uint64(c.countRange(0, int32(lowbits(end))))
break
}
if k > ekey {
break
}
}
return n
}
@ -485,6 +492,11 @@ func (b *Bitmap) Slice() []uint64 {
// SliceRange returns a slice of integers between [start, end).
func (b *Bitmap) SliceRange(start, end uint64) []uint64 {
if roaringParanoia {
if start > end {
panic(fmt.Sprintf("getting slice in range but %v > %v", start, end))
}
}
var a []uint64
itr := b.Iterator()
itr.Seek(start)
@ -934,6 +946,11 @@ func (b *Bitmap) Shift(n int) (*Bitmap, error) {
lastKey := uint64(0)
for iiter.Next() {
ki, ci := iiter.Value()
if lastCarry && ki > lastKey+1 {
extra := NewContainerArray([]uint16{0})
output.Containers.Put(lastKey+1, extra)
lastCarry = false
}
o, carry := shift(ci)
if lastCarry {
o.add(0)
@ -1249,6 +1266,11 @@ func (b *Bitmap) Check() error {
// Flip performs a logical negate of the bits in the range [start,end].
func (b *Bitmap) Flip(start, end uint64) *Bitmap {
if roaringParanoia {
if start > end {
panic(fmt.Sprintf("flipping in range but %v > %v", start, end))
}
}
result := NewBitmap()
itr := b.Iterator()
v, eof := itr.Next()
@ -1310,8 +1332,14 @@ func (itr *Iterator) Seek(seek uint64) {
}
// Move to the correct value index inside the container.
lb := lowbits(seek)
lb, hb := lowbits(seek), highbits(seek)
if itr.c.isArray() {
// Seek is smaller than min(itr.c).
if itr.key > hb {
itr.j = -1
return
}
// Find index in the container.
itr.j = search32(itr.c.array(), lb)
if itr.j < 0 {
@ -1333,24 +1361,37 @@ func (itr *Iterator) Seek(seek uint64) {
}
if itr.c.isRun() {
if seek == 0 {
itr.j, itr.k = 0, -1
// Seek is smaller than min(itr.c).
if itr.key > hb {
itr.j = 0
itr.k = -1
return
}
j, contains := binSearchRuns(lb, itr.c.runs())
if contains {
itr.j = j
itr.k = int32(lb) - int32(itr.c.runs()[j].start) - 1
} else {
// Set iterator to next value in the Bitmap.
itr.j = j
itr.k = -1
return
}
// If seek is larger than all elements, return.
if j >= int32(len(itr.c.runs())) {
if !itr.citer.Next() {
itr.c = nil
return
}
}
// Set iterator to next value in the Bitmap.
itr.j = j
itr.k = -1
return
}
// If it's a bitmap container then move to index before the value and call next().
if itr.key > hb {
itr.j = -1
return
}
itr.j = int32(lb) - 1
}
@ -1502,6 +1543,11 @@ func (c *Container) countRange(start, end int32) (n int32) {
if c == nil {
return 0
}
if roaringParanoia {
if start > end {
panic(fmt.Sprintf("counting in range but %v > %v", start, end))
}
}
if c.isArray() {
return c.arrayCountRange(start, end)
} else if c.isRun() {
@ -1511,6 +1557,11 @@ func (c *Container) countRange(start, end int32) (n int32) {
}
func (c *Container) arrayCountRange(start, end int32) (n int32) {
if roaringParanoia {
if start > end {
panic(fmt.Sprintf("counting in range but %v > %v", start, end))
}
}
array := c.array()
i := int32(sort.Search(len(array), func(i int) bool { return int32(array[i]) >= start }))
for ; i < int32(len(array)); i++ {
@ -1524,6 +1575,11 @@ func (c *Container) arrayCountRange(start, end int32) (n int32) {
}
func (c *Container) bitmapCountRange(start, end int32) int32 {
if roaringParanoia {
if start > end {
panic(fmt.Sprintf("counting in range but %v > %v", start, end))
}
}
var n uint64
i, j := start/64, end/64
// Special case when start and end fall in the same word.
@ -1555,6 +1611,11 @@ func (c *Container) bitmapCountRange(start, end int32) int32 {
}
func (c *Container) runCountRange(start, end int32) (n int32) {
if roaringParanoia {
if start > end {
panic(fmt.Sprintf("counting in range but %v > %v", start, end))
}
}
runs := c.runs()
for _, iv := range runs {
// iv is before range

View file

@ -2025,6 +2025,15 @@ func TestIteratorArray(t *testing.T) {
t.Fatalf("iterator did not next correctly across containers: %v\n", itr)
}
itr.Seek(65535)
if !(itr.key == 1 && itr.j == -1) {
t.Fatalf("iterator did not seek missing value in previous container correctly: %v\n", itr)
}
val, eof = itr.Next()
if !(val == 90000 && !eof) {
t.Fatalf("iterator did not next from missing value in previous container correctly: %d, %v\n", val, eof)
}
itr.Seek(80000)
if !(itr.key == 1 && itr.j == -1) {
t.Fatalf("iterator did not seek missing value correctly: %v\n", itr)
@ -2044,6 +2053,24 @@ func TestIteratorArray(t *testing.T) {
if !(val == 0 && eof) {
t.Fatalf("iterator did not eof correctly: %d, %v\n", val, eof)
}
// Test for seeking value not in bitmap, where next container that the iterator should
// go to has values with low bits smaller than the low bits of seek.
b = NewBitmap(65537, 65538, 65539, 65541, 65542)
itr = b.Iterator()
// Both 65536+5-1 and 5 are not in b.
itr.Seek(5)
if !(itr.key == 1 && itr.j == -1) {
t.Fatalf("iterator did not seek correctly in next container: %v\n", itr)
}
val, eof = itr.Next()
if !(val == 65537 && !eof) {
t.Fatalf("iterator did not next corrrectly to next container: %d, %v\n", val, eof)
}
val, eof = itr.Next()
if !(val == 65538 && !eof) {
t.Fatalf("iterator did not next correctly: %d, %v\n", val, eof)
}
}
func TestIteratorBitmap(t *testing.T) {
@ -2102,6 +2129,39 @@ func TestIteratorBitmap(t *testing.T) {
if !(val == 0 && eof) {
t.Fatalf("iterator did not eof correctly: %d, %v\n", val, eof)
}
// Test for seeking value not in bitmap, where next container that the iterator should
// go to has values with low bits smaller than the low bits of seek.
for i := uint64(65536*3 + 2); i < 65536*3+7; i++ {
if i != 65536*3+5 {
if _, err := b.Add(i); err != nil {
t.Fatalf("adding bit: %v", err)
}
}
}
for i := uint64(65536*3 + 8); i < 65536*3+4110; i++ {
if _, err := b.Add(i); err != nil {
t.Fatalf("adding bit: %v", err)
}
}
if !b.Containers.Get(3).isBitmap() {
t.Fatalf("wrong container type")
}
// Both 65536*2+5 and 65536*3+5 are not in b.
itr.Seek(65536*2 + 5)
if !(itr.key == 3 && itr.j == -1) {
t.Fatalf("iterator did not seek correctly in next container: %v\n", itr)
}
val, eof = itr.Next()
if !((val == 65536*3+2) && !eof) {
t.Fatalf("iterator did not next correctly to next container: %d, %v\n", val, eof)
}
val, eof = itr.Next()
if !((val == 65536*3+3) && !eof) {
t.Fatalf("iterator did not next correctly to next container: %d, %v\n", val, eof)
}
}
func TestIteratorRuns(t *testing.T) {
@ -2164,6 +2224,38 @@ func TestIteratorRuns(t *testing.T) {
if !(val == 0 && eof) {
t.Fatalf("iterator did not eof correctly: %d, %v\n", val, eof)
}
// Test for seeking value not in bitmap, where next container that the iterator should
// go to has values with low bits smaller than the low bits of seek.
for i := uint64(65536*3 + 1); i <= 65536*3+8; i++ {
if _, err := b.Add(i); err != nil {
t.Fatalf("adding bit: %v", err)
}
}
for i := uint64(65536*3 + 10); i <= 65536*3+20; i++ {
if _, err := b.Add(i); err != nil {
t.Fatalf("adding bit: %v", err)
}
}
b.Optimize()
if !b.Containers.Get(3).isRun() {
t.Fatalf("wrong container type")
}
// Both 65536*2+9 and 65536*3+9 are not in b.
itr.Seek(65536*2 + 9)
if !(itr.key == 3 && itr.j == 0 && itr.k == -1) {
t.Fatalf("iterator did not seek correctly in next container: %v\n", itr)
}
val, eof = itr.Next()
if !((val == 65536*3+1) && !eof) {
t.Fatalf("iterator did not next correctly to next container: %d, %v\n", val, eof)
}
val, eof = itr.Next()
if !((val == 65536*3+2) && !eof) {
t.Fatalf("iterator did not next correctly to next container: %d, %v\n", val, eof)
}
}
func TestIteratorVarious(t *testing.T) {

View file

@ -146,6 +146,20 @@ func TestCountRange(t *testing.T) {
end: 6 * 65536,
exp: 1,
},
{
name: "start < end in different containers",
bitmap: []uint64{65537, 65538, 65539, 65540},
start: 65536,
end: 2,
exp: 0,
},
{
name: "start == end",
bitmap: []uint64{65537, 65538, 65539, 65540},
start: 65537,
end: 65537,
exp: 0,
},
}
for _, test := range tests {
@ -390,6 +404,11 @@ func TestBitmap_BitmapCountRange(t *testing.T) {
if n := bm0.CountRange(10000000, 10000001); n != 0 {
t.Fatalf("unexpected n: %d", n)
}
// Test the case where start < end and bitmap contains value in the same container as start.
if n := bm0.CountRange(65536, 2); n != 0 {
t.Fatalf("unexpected n: %d", n)
}
}
func TestBitmap_ArrayCountRange(t *testing.T) {
@ -397,6 +416,11 @@ func TestBitmap_ArrayCountRange(t *testing.T) {
if n := bm0.CountRange(1, 2683313); n != 1 {
t.Fatalf("unexpected n: %d", n)
}
// Test the case where start < end and bitmap contains value in the same container as start.
if n := bm0.CountRange(2621440, 2); n != 0 {
t.Fatalf("unexpected n: %d", n)
}
}
func TestBitmap_DirectAdd(t *testing.T) {
@ -427,6 +451,12 @@ func TestBitmap_RunCountRange(t *testing.T) {
if n := bm1.CountRange(5, 12); n != 7 {
t.Fatalf("unexpected n: %d", n)
}
bm2 := roaring.NewFileBitmap(65536, 65537, 65538, 65539, 65540, 65541, 65542, 65543, 65544, 65545, 65546, 65547, 65548, 65549, 65550, 65551, 65552, 65553)
bm2.Optimize() // convert to runs
if n := bm2.CountRange(3, 2); n != 0 {
t.Fatalf("unexpected n: %d", n)
}
}
func TestBitmap_Intersection(t *testing.T) {
@ -1063,6 +1093,22 @@ func TestBitmap_Shift(t *testing.T) {
} else if !reflect.DeepEqual(got.Slice(), bm2.Slice()) {
t.Fatalf("unexpected bitmap: expected %v, but got %v", bm2.Slice(), got.Slice())
}
bm1 = roaring.NewFileBitmap(65535, 131073)
bm2 = roaring.NewFileBitmap(65536, 131074)
if got, err := bm1.Shift(1); err != nil {
t.Fatal(err)
} else if !reflect.DeepEqual(got.Slice(), bm2.Slice()) {
t.Fatalf("unexpected bitmap: expected %v, but got %v", bm2.Slice(), got.Slice())
}
bm1 = roaring.NewFileBitmap(65535, 131073, 65536*5-1, 65536*10, 65536*15-1)
bm2 = roaring.NewFileBitmap(65536, 131074, 65536*5, 65536*10+1, 65536*15)
if got, err := bm1.Shift(1); err != nil {
t.Fatal(err)
} else if !reflect.DeepEqual(got.Slice(), bm2.Slice()) {
t.Fatalf("unexpected bitmap: expected %v, but got %v", bm2.Slice(), got.Slice())
}
}
func TestBitmap_Quick_Array1(t *testing.T) { testBitmapQuick(t, 1000, 1000, 2000) }
@ -1078,17 +1124,17 @@ func testBitmapQuick(t *testing.T, n int, min, max uint64) {
m := make(map[uint64]struct{})
// Add values to the bitmap and set.
manual_count := uint64(0)
manualCount := uint64(0)
for _, v := range a {
new_bit, _ := bm.Add(v)
if new_bit {
manual_count++
newBit, _ := bm.Add(v)
if newBit {
manualCount++
}
m[v] = struct{}{}
}
//check count
if manual_count != bm.Count() {
t.Fatalf("expected bitmap Add count to be: %d got: %d", manual_count, bm.Count())
if manualCount != bm.Count() {
t.Fatalf("expected bitmap Add count to be: %d got: %d", manualCount, bm.Count())
}
// Verify existence.
@ -1116,12 +1162,12 @@ func testBitmapQuick(t *testing.T, n int, min, max uint64) {
for i, item := range rand.Perm(len(a)) {
removed, _ := bm.Remove(a[item])
if removed {
manual_count--
manualCount--
}
//check count
if manual_count != bm.Count() {
if manualCount != bm.Count() {
t.Fatalf("removing %d/%d [%d] from bitmap: expected bitmap Remove count to be %d, got %d",
i, len(a), a[item], manual_count, bm.Count())
i, len(a), a[item], manualCount, bm.Count())
}
}
@ -1241,13 +1287,13 @@ func TestIterator(t *testing.T) {
t.Run("run", func(t *testing.T) {
bm1 := roaring.NewFileBitmap()
for i := uint64(0); i < 11; i += 1 {
for i := uint64(0); i < 11; i++ {
_, _ = bm1.Add(i)
}
bm1.Optimize()
bm2 := roaring.NewFileBitmap()
for i := uint64(0); i < 12; i += 1 {
for i := uint64(0); i < 12; i++ {
_, _ = bm2.Add(i)
}
bm2.Optimize()
@ -1290,11 +1336,11 @@ func testBM() *roaring.Bitmap {
_, _ = bm.Add((2 << 16) + i)
}
//small run
for i := uint64(0); i < 1024; i += 1 {
for i := uint64(0); i < 1024; i++ {
_, _ = bm.Add((3 << 16) + i)
}
//large run
for i := uint64(0); i < 65535; i += 1 {
for i := uint64(0); i < 65535; i++ {
_, _ = bm.Add((4 << 16) + i)
}
bm.Optimize()