fix a bunch of bugs and add some tests

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)
 }

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")
+	}
+
+}

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
+}

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)

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++ {