diff --git a/roaring/containers.go b/roaring/containers.go index 467430c74..c80dbeb06 100644 --- a/roaring/containers.go +++ b/roaring/containers.go @@ -1,8 +1,6 @@ package roaring -import ( - "github.com/pilosa/fast-skiplist" -) +import "github.com/pilosa/fast-skiplist" func NewSkipListContainers() *SkipListContainers { return &SkipListContainers{ @@ -15,7 +13,12 @@ type SkipListContainers struct { } func (slc *SkipListContainers) Get(key uint64) *container { - return slc.list.Get(key).Value().(*container) + var c *container + el := slc.list.Get(key) + if el != nil { + c = el.Value().(*container) + } + return c } func (slc *SkipListContainers) Put(key uint64, c *container) { @@ -43,6 +46,9 @@ func (slc *SkipListContainers) Clone() Containers { } func (slc *SkipListContainers) Last() (key uint64, c *container) { + if slc.list.Length() == 0 { + return 0, nil + } el := slc.list.Last() return el.Key(), el.Value().(*container) } @@ -53,14 +59,13 @@ func (slc *SkipListContainers) Size() int { func (slc *SkipListContainers) Iterator(key uint64) (citer Contiterator, found bool) { el := slc.list.GetNext(key) - if el.Key() == key { + if el != nil && el.Key() == key { found = true } return &SLCIterator{ el: el, }, found - } type SLCIterator struct { @@ -81,5 +86,8 @@ func (i *SLCIterator) Next() bool { } func (i *SLCIterator) Value() (uint64, *container) { + if !i.started || i.el == nil { + return 0, nil + } return i.el.Key(), i.el.Value().(*container) } diff --git a/roaring/containers_test.go b/roaring/containers_test.go new file mode 100644 index 000000000..464a92fd6 --- /dev/null +++ b/roaring/containers_test.go @@ -0,0 +1,84 @@ +package roaring + +import ( + "testing" +) + +func TestContainersIterator(t *testing.T) { + slc := NewSkipListContainers() + itr, found := slc.Iterator(0) + if found { + t.Fatalf("shouldn't have found 0 in empty slc") + } + if itr.Next() { + t.Fatal("Next() should be false for empty slc") + } + + slc.Put(1, &container{n: 1}) + slc.Put(2, &container{n: 2}) + + itr, found = slc.Iterator(0) + if found { + t.Fatalf("shouldn't have found 0") + } + + if !itr.Next() { + t.Fatalf("one should be next, but got false") + } + if key, val := itr.Value(); key != 1 || val.n != 1 { + t.Fatalf("Wrong k/v, exp: 1,1 got: %v,%v", key, val.n) + } + if !itr.Next() { + t.Fatalf("two should be next, but got false") + } + if key, val := itr.Value(); key != 2 || val.n != 2 { + t.Fatalf("Wrong k/v, exp: 2,2 got: %v,%v", key, val.n) + } + + if itr.Next() { + t.Fatalf("itr should be done, but got true") + } + + slc.Put(3, &container{n: 3}) + slc.Put(5, &container{n: 5}) + slc.Put(6, &container{n: 6}) + + itr, found = slc.Iterator(3) + if !itr.Next() { + t.Fatalf("3 should be next, but got false") + } + if !found { + t.Fatalf("should have found 3") + } + if key, val := itr.Value(); key != 3 || val.n != 3 { + t.Fatalf("Wrong k/v, exp: 3,3 got: %v,%v", key, val.n) + } + if !itr.Next() { + t.Fatalf("5 should be next, but got false") + } + if key, val := itr.Value(); key != 5 || val.n != 5 { + t.Fatalf("Wrong k/v, exp: 5,5 got: %v,%v", key, val.n) + } + + itr, found = slc.Iterator(4) + if found { + t.Fatalf("shouldn't have found 4") + } + if !itr.Next() { + t.Fatalf("5 should be next, but got false") + } + if key, val := itr.Value(); key != 5 || val.n != 5 { + t.Fatalf("Wrong k/v, exp: 5,5 got: %v,%v", key, val.n) + } + if !itr.Next() { + t.Fatalf("6 should be next, but got false") + } + if key, val := itr.Value(); key != 6 || val.n != 6 { + t.Fatalf("Wrong k/v, exp: 6,6 got: %v,%v", key, val.n) + } + + if itr.Next() { + t.Fatalf("itr should be done, but got true") + } + +} diff --git a/roaring/roaring.go b/roaring/roaring.go index 6fa352a12..2de14caf3 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -21,6 +21,7 @@ import ( "fmt" "hash/fnv" "io" + "reflect" "sort" "unsafe" ) @@ -108,7 +109,9 @@ type Bitmap struct { // NewBitmap returns a Bitmap with an initial set of values. func NewBitmap(a ...uint64) *Bitmap { - b := &Bitmap{} + b := &Bitmap{ + conts: NewSkipListContainers(), + } b.Add(a...) return b } @@ -263,6 +266,7 @@ func (b *Bitmap) Slice() []uint64 { var a []uint64 itr := b.Iterator() itr.Seek(0) + for v, eof := itr.Next(); !eof; v, eof = itr.Next() { a = append(a, v) } @@ -313,7 +317,7 @@ func (b *Bitmap) OffsetRange(offset, start, end uint64) *Bitmap { off := highbits(offset) hi0, hi1 := highbits(start), highbits(end) citer, _ := b.conts.Iterator(hi0) - var other Bitmap + other := NewBitmap() for citer.Next() { k, c := citer.Value() if k >= hi1 { @@ -321,7 +325,7 @@ func (b *Bitmap) OffsetRange(offset, start, end uint64) *Bitmap { } other.conts.Put(off+(k-hi0), c) } - return &other + return other } // container returns the container with the given key. @@ -335,7 +339,7 @@ func (b *Bitmap) container(key uint64) *container { func (b *Bitmap) IntersectionCount(other *Bitmap) uint64 { var n uint64 iiter, _ := b.conts.Iterator(0) - jiter, _ := b.conts.Iterator(0) + jiter, _ := other.conts.Iterator(0) i, j := iiter.Next(), jiter.Next() ki, ci := iiter.Value() kj, cj := jiter.Value() @@ -358,9 +362,14 @@ func (b *Bitmap) IntersectionCount(other *Bitmap) uint64 { // Intersect returns the intersection of b and other. func (b *Bitmap) Intersect(other *Bitmap) *Bitmap { +<<<<<<< 49aec2dc4c2137c459c5ff4902609b93cfc72933 output := &Bitmap{} +======= + output := NewBitmap() + +>>>>>>> fix a bunch of bugs and add some tests iiter, _ := b.conts.Iterator(0) - jiter, _ := b.conts.Iterator(0) + jiter, _ := other.conts.Iterator(0) i, j := iiter.Next(), jiter.Next() ki, ci := iiter.Value() kj, cj := jiter.Value() @@ -383,19 +392,19 @@ func (b *Bitmap) Intersect(other *Bitmap) *Bitmap { // Union returns the bitwise union of b and other. func (b *Bitmap) Union(other *Bitmap) *Bitmap { - output := &Bitmap{} + output := NewBitmap() iiter, _ := b.conts.Iterator(0) - jiter, _ := b.conts.Iterator(0) + jiter, _ := other.conts.Iterator(0) i, j := iiter.Next(), jiter.Next() ki, ci := iiter.Value() kj, cj := jiter.Value() for i || j { - if !j || ki < kj { + if i && (!j || ki < kj) { output.conts.Put(ki, ci.clone()) i = iiter.Next() ki, ci = iiter.Value() - } else if !i || ki > kj { + } else if j && (!i || ki > kj) { output.conts.Put(kj, cj.clone()) j = jiter.Next() kj, cj = jiter.Value() @@ -411,19 +420,19 @@ func (b *Bitmap) Union(other *Bitmap) *Bitmap { // Difference returns the difference of b and other. func (b *Bitmap) Difference(other *Bitmap) *Bitmap { - output := &Bitmap{} + output := NewBitmap() iiter, _ := b.conts.Iterator(0) - jiter, _ := b.conts.Iterator(0) + jiter, _ := other.conts.Iterator(0) i, j := iiter.Next(), jiter.Next() ki, ci := iiter.Value() kj, cj := jiter.Value() for i || j { - if !j || ki < kj { + if i && (!j || ki < kj) { output.conts.Put(ki, ci.clone()) i = iiter.Next() ki, ci = iiter.Value() - } else if !i || ki > kj { + } else if j && (!i || ki > kj) { j = jiter.Next() kj, cj = jiter.Value() } else { // ki == kj @@ -438,19 +447,19 @@ func (b *Bitmap) Difference(other *Bitmap) *Bitmap { // Xor returns the bitwise exclusive or of b and other. func (b *Bitmap) Xor(other *Bitmap) *Bitmap { - output := &Bitmap{} + output := NewBitmap() iiter, _ := b.conts.Iterator(0) - jiter, _ := b.conts.Iterator(0) + jiter, _ := other.conts.Iterator(0) i, j := iiter.Next(), jiter.Next() ki, ci := iiter.Value() kj, cj := jiter.Value() for i || j { - if !j || ki < kj { + if i && (!j || ki < kj) { output.conts.Put(ki, ci.clone()) i = iiter.Next() ki, ci = iiter.Value() - } else if !i || ki > kj { + } else if j && (!i || ki > kj) { output.conts.Put(kj, cj.clone()) j = jiter.Next() kj, cj = jiter.Value() @@ -800,7 +809,7 @@ func (itr *Iterator) Seek(seek uint64) { itr.k = -1 // Move to the correct container. - itr.citer, _ = itr.bitmap.conts.Iterator(seek) + itr.citer, _ = itr.bitmap.conts.Iterator(highbits(seek)) if !itr.citer.Next() { itr.c = nil return // eof @@ -856,7 +865,7 @@ func (itr *Iterator) Seek(seek uint64) { // Returns eof as true if there are no values left in the iterator. func (itr *Iterator) Next() (v uint64, eof bool) { if itr.c == nil { - return + return 0, true } // Iterate over containers until we find the next value or EOF. for { @@ -3223,3 +3232,31 @@ func xorBitmapRun(a, b *container) *container { } return output } + +func BitmapsEqual(b, c *Bitmap) error { + if b.OpWriter != c.OpWriter { + return errors.New("opWriters not equal") + } + if b.opN != c.opN { + return errors.New("opNs not equal") + } + + biter, _ := b.conts.Iterator(0) + citer, _ := c.conts.Iterator(0) + bn, cn := biter.Next(), citer.Next() + for ; bn && cn; bn, cn = biter.Next(), citer.Next() { + bk, bc := biter.Value() + ck, cc := citer.Value() + if bk != ck { + return errors.New("keys not equal") + } + if !reflect.DeepEqual(bc, cc) { + return errors.New("containers not equal") + } + } + if bn && !cn || cn && !bn { + return errors.New("different numbers of containers") + } + + return nil +} diff --git a/roaring/roaring_internal_test.go b/roaring/roaring_internal_test.go index 268e02de4..7503ab247 100644 --- a/roaring/roaring_internal_test.go +++ b/roaring/roaring_internal_test.go @@ -1714,9 +1714,9 @@ func TestDifferenceRunRun(t *testing.T) { func TestWriteReadArray(t *testing.T) { ca := &container{array: []uint16{1, 10, 100, 1000}, n: 4, containerType: ContainerArray} - ba := &Bitmap{} + ba := NewBitmap() ba.conts.Put(0, ca) - ba2 := &Bitmap{} + ba2 := NewBitmap() var buf bytes.Buffer _, err := ba.WriteTo(&buf) if err != nil { @@ -1737,9 +1737,9 @@ func TestWriteReadBitmap(t *testing.T) { for i := 0; i < 129; i++ { cb.bitmap[i] = 0x5555555555555555 } - bb := &Bitmap{} + bb := NewBitmap() bb.conts.Put(0, cb) - bb2 := &Bitmap{} + bb2 := NewBitmap() var buf bytes.Buffer _, err := bb.WriteTo(&buf) if err != nil { @@ -1760,9 +1760,9 @@ func TestWriteReadFullBitmap(t *testing.T) { for i := 0; i < bitmapN; i++ { cb.bitmap[i] = 0xffffffffffffffff } - bb := &Bitmap{} + bb := NewBitmap() bb.conts.Put(0, cb) - bb2 := &Bitmap{} + bb2 := NewBitmap() var buf bytes.Buffer _, err := bb.WriteTo(&buf) if err != nil { @@ -1786,9 +1786,9 @@ func TestWriteReadFullBitmap(t *testing.T) { func TestWriteReadRun(t *testing.T) { cr := &container{runs: []interval16{{start: 3, last: 13}, {start: 100, last: 109}}, n: 21, containerType: ContainerRun} - br := &Bitmap{} + br := NewBitmap() br.conts.Put(0, cr) - br2 := &Bitmap{} + br2 := NewBitmap() var buf bytes.Buffer _, err := br.WriteTo(&buf) if err != nil { @@ -2101,7 +2101,7 @@ func TestIteratorArray(t *testing.T) { itr.Seek(1000) if !(itr.key == 0 && itr.j == 3) { - t.Fatalf("iterator did not seek correctly: %v\n", itr) + t.Fatalf("iterator did not seek correctly: %#v\n", itr) } itr.Seek(10000) diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go index 6378ee67f..f972d8573 100644 --- a/roaring/roaring_test.go +++ b/roaring/roaring_test.go @@ -35,8 +35,8 @@ func TestBitmapClone(t *testing.T) { b.Add(i) } c := b.Clone() - if !reflect.DeepEqual(b, c) { - t.Fatalf("Clone Objects not equal\n") + if err := roaring.BitmapsEqual(b, c); err != nil { + t.Fatalf("Clone Objects not equal: %v\n", err) } d := func() *roaring.Bitmap { //anybody know how to declare a nil value? return nil @@ -363,7 +363,7 @@ func TestBitmap_RunCountRange(t *testing.T) { } } -func TestBitmap_Intersection(t *testing.T) { +func TestBitmap_Intersectionz(t *testing.T) { bm0 := roaring.NewBitmap(0, 2683177) bm1 := roaring.NewBitmap() for i := uint64(628); i < 2683301; i++ {