From 8fd752ddb14461896d03c210142742060ab68b20 Mon Sep 17 00:00:00 2001 From: Travis Date: Thu, 22 Jun 2017 16:01:52 -0500 Subject: [PATCH 1/2] bug fix in `differenceArrayRun` logic. add more test coverage for `differenceArrayRun` --- roaring/roaring.go | 33 ++++++++++++++++++++------------- roaring/roaring_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 66ce116eb..653831940 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -453,7 +453,6 @@ func (b *Bitmap) Difference(other *Bitmap) *Bitmap { } } - return output } @@ -2488,29 +2487,37 @@ func differenceArrayRun(a, b *container) *container { i := 0 // array index j := 0 // run index - // keep all array elements before beginning of runs - for ; i < len(a.array) && a.array[i] < b.runs[j].start; i++ { - output.array = append(output.array, a.array[i]) - } - // handle overlap - for ; i < a.n; i++ { - // if array element in run, keep - if !(a.array[i] >= b.runs[j].start && a.array[i] <= b.runs[j].last) { - output.array = append(output.array, a.array[i]) + for i < a.n { + + // keep all array elements before beginning of runs + if a.array[i] < b.runs[j].start { + output.add(a.array[i]) + i++ + continue } - // update current run - if a.array[i] >= b.runs[j].last { + + // if array element in run, skip it + if a.array[i] >= b.runs[j].start && a.array[i] <= b.runs[j].last { + i++ + continue + } + + // if array element larger than current run, check next run + if a.array[i] > b.runs[j].last { j++ if j == len(b.runs) { break } } } - i++ + if i < len(a.array) { // keep all array elements after end of runs output.array = append(output.array, a.array[i:]...) + // TODO: consider handling container.n mutations in one place + // like we do with container.add(). + output.n += len(a.array[i:]) } return output } diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go index a097e1fbe..fc0a48966 100644 --- a/roaring/roaring_test.go +++ b/roaring/roaring_test.go @@ -242,6 +242,7 @@ func TestBitmap_Difference(t *testing.T) { t.Fatalf("unexpected n: %d", n) } } + func TestBitmap_Difference_Empty(t *testing.T) { bm0 := roaring.NewBitmap(0, 2683177) bm1 := roaring.NewBitmap() @@ -251,6 +252,33 @@ func TestBitmap_Difference_Empty(t *testing.T) { } } +func TestBitmap_DifferenceArrayArray(t *testing.T) { + bm0 := roaring.NewBitmap(0, 4, 8, 12, 16, 20) + bm1 := roaring.NewBitmap(1, 3, 6, 9, 12, 15, 18) + result := bm0.Difference(bm1) + if n := result.Count(); n != 5 { + t.Fatalf("unexpected n: %d", n) + } +} + +func TestBitmap_DifferenceArrayRun(t *testing.T) { + bm0 := roaring.NewBitmap(0, 4, 8, 12, 16, 20, 36, 40, 44) + + bm1 := roaring.NewBitmap(1, 2, 3, 4, 5, 6, 7, 8, 9, 30, 31, 32, 33, 34, 35, 36) + bm1.Optimize() // convert to runs + result := bm0.Difference(bm1) + if n := result.Count(); n != 6 { + t.Fatalf("unexpected n: %d", n) + } + + // ensure empty array returns empty + bm2 := roaring.NewBitmap() + result = bm2.Difference(bm1) + if n := result.Count(); n != 0 { + t.Fatalf("unexpected n: %d", n) + } +} + func TestBitmap_Union(t *testing.T) { bm0 := roaring.NewBitmap(0, 1000001, 1000002, 1000003) bm1 := roaring.NewBitmap(0, 50000, 1000001, 1000002) From fc6898a9337b40432be1da1b23c14d4fd1037dae Mon Sep 17 00:00:00 2001 From: Travis Date: Thu, 22 Jun 2017 16:07:21 -0500 Subject: [PATCH 2/2] remove a test that was not testing the correct thing --- roaring/roaring_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/roaring/roaring_test.go b/roaring/roaring_test.go index fc0a48966..98c789043 100644 --- a/roaring/roaring_test.go +++ b/roaring/roaring_test.go @@ -270,13 +270,6 @@ func TestBitmap_DifferenceArrayRun(t *testing.T) { if n := result.Count(); n != 6 { t.Fatalf("unexpected n: %d", n) } - - // ensure empty array returns empty - bm2 := roaring.NewBitmap() - result = bm2.Difference(bm1) - if n := result.Count(); n != 0 { - t.Fatalf("unexpected n: %d", n) - } } func TestBitmap_Union(t *testing.T) {