From d93a6b486b22dd154ea9feee2796259d384e3991 Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Sat, 20 May 2023 17:01:46 -0400 Subject: [PATCH] Verifying issue 386 --- roaring64/roaring64cow_test.go | 29 +++++++++++++++++++++++++++++ roaring64/roaringarray64.go | 13 ++++++++----- roaringcow_test.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/roaring64/roaring64cow_test.go b/roaring64/roaring64cow_test.go index c88c9c05..b3b7bee9 100644 --- a/roaring64/roaring64cow_test.go +++ b/roaring64/roaring64cow_test.go @@ -11,6 +11,35 @@ import ( "github.com/stretchr/testify/assert" ) +func TestIssue386(t *testing.T) { + a := NewBitmap() + for i := uint64(0); i < 1000; i++ { + a.Add(i) + } + a.SetCopyOnWrite(true) + aClone := a.Clone() + assert.False(t, a.Contains(1001)) + assert.EqualValues(t, 1000, a.GetCardinality()) + assert.EqualValues(t, 1000, aClone.GetCardinality()) + b := NewBitmap() + b.Add(1001) + assert.False(t, a.Contains(1001)) + assert.True(t, b.Contains(1001)) + assert.EqualValues(t, 1000, a.GetCardinality()) + assert.EqualValues(t, 1000, aClone.GetCardinality()) + c := NewBitmap() + c.Or(aClone) + assert.EqualValues(t, 1000, c.GetCardinality()) + assert.EqualValues(t, 1000, a.GetCardinality()) + assert.EqualValues(t, 1000, aClone.GetCardinality()) + c.Or(b) + assert.EqualValues(t, 1001, c.GetCardinality()) + assert.True(t, c.Contains(1001)) + assert.False(t, a.Contains(1001)) + assert.EqualValues(t, 1000, a.GetCardinality()) + assert.EqualValues(t, 1000, aClone.GetCardinality()) +} + func TestCloneOfCOW(t *testing.T) { rb1 := NewBitmap() rb1.SetCopyOnWrite(true) diff --git a/roaring64/roaringarray64.go b/roaring64/roaringarray64.go index 9e3b34bc..446c1884 100644 --- a/roaring64/roaringarray64.go +++ b/roaring64/roaringarray64.go @@ -1,6 +1,8 @@ package roaring64 -import "github.com/RoaringBitmap/roaring" +import ( + "github.com/RoaringBitmap/roaring" +) type roaringArray64 struct { keys []uint32 @@ -12,9 +14,10 @@ type roaringArray64 struct { // runOptimize compresses the element containers to minimize space consumed. // Q: how does this interact with copyOnWrite and needCopyOnWrite? // A: since we aren't changing the logical content, just the representation, -// we don't bother to check the needCopyOnWrite bits. We replace -// (possibly all) elements of ra.containers in-place with space -// optimized versions. +// +// we don't bother to check the needCopyOnWrite bits. We replace +// (possibly all) elements of ra.containers in-place with space +// optimized versions. func (ra *roaringArray64) runOptimize() { for i := range ra.containers { ra.containers[i].RunOptimize() @@ -39,7 +42,7 @@ func (ra *roaringArray64) appendCopy(sa roaringArray64, startingindex int) { // since there is no copy-on-write, we need to clone the container (this is important) ra.appendContainer(sa.keys[startingindex], sa.containers[startingindex].Clone(), copyonwrite) } else { - ra.appendContainer(sa.keys[startingindex], sa.containers[startingindex], copyonwrite) + ra.appendContainer(sa.keys[startingindex], sa.containers[startingindex].Clone(), copyonwrite) if !sa.needsCopyOnWrite(startingindex) { sa.setNeedsCopyOnWrite(startingindex) } diff --git a/roaringcow_test.go b/roaringcow_test.go index 09573804..94d07acb 100644 --- a/roaringcow_test.go +++ b/roaringcow_test.go @@ -10,6 +10,35 @@ import ( "github.com/stretchr/testify/assert" ) +func TestIssue386(t *testing.T) { + a := NewBitmap() + for i := uint32(0); i < 1000; i++ { + a.Add(i) + } + a.SetCopyOnWrite(true) + aClone := a.Clone() + assert.False(t, a.Contains(1001)) + assert.EqualValues(t, 1000, a.GetCardinality()) + assert.EqualValues(t, 1000, aClone.GetCardinality()) + b := NewBitmap() + b.Add(1001) + assert.False(t, a.Contains(1001)) + assert.True(t, b.Contains(1001)) + assert.EqualValues(t, 1000, a.GetCardinality()) + assert.EqualValues(t, 1000, aClone.GetCardinality()) + c := NewBitmap() + c.Or(aClone) + assert.EqualValues(t, 1000, c.GetCardinality()) + assert.EqualValues(t, 1000, a.GetCardinality()) + assert.EqualValues(t, 1000, aClone.GetCardinality()) + c.Or(b) + assert.EqualValues(t, 1001, c.GetCardinality()) + assert.True(t, c.Contains(1001)) + assert.False(t, a.Contains(1001)) + assert.EqualValues(t, 1000, a.GetCardinality()) + assert.EqualValues(t, 1000, aClone.GetCardinality()) +} + func TestCloneOfCOW(t *testing.T) { rb1 := NewBitmap() rb1.SetCopyOnWrite(true)