From 40b27b66438e28e9d514242d854b0b3da02ba2f7 Mon Sep 17 00:00:00 2001 From: Travis Date: Mon, 25 Sep 2017 12:34:15 -0500 Subject: [PATCH] Fix panic when iterating over an empty run container. When the difference of two run containers resulted in an empty container, that container would be a run container with no runs. The Iterator was expected there to be at least on run in `container.runs`. This fix protects against that and adds tests for that case. --- roaring/roaring.go | 8 +++++- roaring/roaring_test.go | 56 ++++++++++++++++++++++++++++++++++------- 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index e10b6a99e..b4b9fc206 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -444,7 +444,6 @@ func (b *Bitmap) Difference(other *Bitmap) *Bitmap { output.keys = append(output.keys, key) output.containers = append(output.containers, container) } - } return output } @@ -893,6 +892,13 @@ func (itr *Iterator) Next() (v uint64, eof bool) { if itr.j == -1 { itr.j++ } + + // If the container is empty, move to the next container. + if len(c.runs) == 0 { + itr.i, itr.j = itr.i+1, -1 + continue + } + r := c.runs[itr.j] runLength := int(r.last - r.start) diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go index a5ac7a30f..f6ac33036 100644 --- a/roaring/roaring_test.go +++ b/roaring/roaring_test.go @@ -951,17 +951,55 @@ func testBitmapMarshalQuick(t *testing.T, n int, min, max uint64, sorted bool) { // Ensure iterator can iterate over all the values on the bitmap. // TODO duplicate for all container types func TestIterator(t *testing.T) { - itr := roaring.NewBitmap(1, 2, 3).Iterator() - itr.Seek(0) + t.Run("bitmap", func(t *testing.T) { + itr := roaring.NewBitmap(1, 2, 3).Iterator() + itr.Seek(0) - var a []uint64 - for v, eof := itr.Next(); !eof; v, eof = itr.Next() { - a = append(a, v) - } + var a []uint64 + for v, eof := itr.Next(); !eof; v, eof = itr.Next() { + a = append(a, v) + } - if !reflect.DeepEqual(a, []uint64{1, 2, 3}) { - t.Fatalf("unexpected values: %+v", a) - } + if !reflect.DeepEqual(a, []uint64{1, 2, 3}) { + t.Fatalf("unexpected values: %+v", a) + } + }) + + t.Run("run", func(t *testing.T) { + bm1 := roaring.NewBitmap() + for i := uint64(0); i < 11; i += 1 { + bm1.Add(i) + } + bm1.Optimize() + + bm2 := roaring.NewBitmap() + for i := uint64(0); i < 12; i += 1 { + bm2.Add(i) + } + bm2.Optimize() + + for _, tt := range []struct { + bm *roaring.Bitmap + expected []uint64 + }{ + {bm1, []uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}}, + {bm2, []uint64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}}, + {bm1.Difference(bm2), []uint64{}}, + {bm2.Difference(bm1), []uint64{11}}, + } { + itr := tt.bm.Iterator() + itr.Seek(0) + + a := []uint64{} + for v, eof := itr.Next(); !eof; v, eof = itr.Next() { + a = append(a, v) + } + + if !reflect.DeepEqual(a, tt.expected) { + t.Fatalf("unexpected values: %#v %#v", a, tt.expected) + } + } + }) } // testBM creates a bitmap with 3 containers: array, bitmap, and run.