From 023faebd90f13b3d68b16765eb914c9b5b166784 Mon Sep 17 00:00:00 2001
From: Matt Jaffee <matthew.jaffee@gmail.com>
Date: Wed, 27 Feb 2019 14:37:33 -0600
Subject: [PATCH] fix bug where opN wasn't getting set/cleared correctly

---
 fragment.go               |  7 ++++++
 fragment_internal_test.go | 50 +++++++++++++++++++++++++++++++++++++++
 roaring/roaring.go        | 21 +++++++++++++---
 3 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/fragment.go b/fragment.go
index 92b57504c..5d89f4815 100644
--- a/fragment.go
+++ b/fragment.go
@@ -238,6 +238,8 @@ func (f *fragment) openStorage() error {
 		return fmt.Errorf("unmarshal storage: file=%s, err=%s", f.file.Name(), err)
 	}
 
+	f.opN = f.storage.Info().OpN
+
 	// Attach the file to the bitmap to act as a write-ahead log.
 	f.storage.OpWriter = f.file
 	f.rowCache = &simpleCache{make(map[uint64]*Row)}
@@ -340,6 +342,11 @@ func (f *fragment) closeStorage() error {
 		}
 	}
 
+	// opN is determined by how many bit set/clear operations are in the storage
+	// write log, so once the storage is closed it should be 0. Opening new
+	// storage will set opN appropriately.
+	f.opN = 0
+
 	return nil
 }
 
diff --git a/fragment_internal_test.go b/fragment_internal_test.go
index 6ed91f707..f706e6738 100644
--- a/fragment_internal_test.go
+++ b/fragment_internal_test.go
@@ -2782,3 +2782,53 @@ func TestFragmentPositionsForValue(t *testing.T) {
 		})
 	}
 }
+
+func TestSmallImportRestart(t *testing.T) {
+	f := mustOpenFragment("i", "f", viewStandard, 0, "")
+	err := f.bulkImport([]uint64{1, 2, 3, 4, 5, 6, 7, 8, 9}, []uint64{1, 2, 3, 4, 5, 6, 7, 8, 9}, &ImportOptions{})
+	if err != nil {
+		t.Fatalf("initial small import: %v", err)
+	}
+	if f.opN != 9 {
+		t.Errorf("unexpected opN - %d is not 9", f.opN)
+	}
+
+	err = f.Close()
+	if err != nil {
+		t.Fatalf("closing fragment: %v", err)
+	}
+
+	err = f.Open()
+	if err != nil {
+		t.Fatalf("reopening fragment: %v", err)
+	}
+
+	if f.opN != 9 {
+		t.Errorf("unexpected opN after close/open %d is not 9", f.opN)
+	}
+
+	if r1 := f.row(1).Columns(); len(r1) != 1 || r1[0] != 1 {
+		t.Errorf("row 1 should be [1], but got %v", r1)
+	}
+
+	f2 := newFragment(f.path, "i", "f", viewStandard, 0)
+	f2.CacheType = f.CacheType
+
+	err = f.closeStorage()
+	if err != nil {
+		t.Fatalf("closing storage: %v", err)
+	}
+
+	err = f2.Open()
+	if err != nil {
+		t.Fatalf("opening new fragment: %v", err)
+	}
+
+	if f2.opN != 9 {
+		t.Errorf("unexpected opN after close/open %d is not 9", f2.opN)
+	}
+
+	if r1 := f2.row(1).Columns(); len(r1) != 1 || r1[0] != 1 {
+		t.Errorf("row 1 should be [1], but got %v", r1)
+	}
+}
diff --git a/roaring/roaring.go b/roaring/roaring.go
index c8eb42016..701f86925 100644
--- a/roaring/roaring.go
+++ b/roaring/roaring.go
@@ -119,7 +119,9 @@ type ContainerIterator interface {
 type Bitmap struct {
 	Containers Containers
 
-	// Number of operations written to the writer.
+	// Number of bit change operations written to the writer. Some operations
+	// contain multiple values, each of those counts the number of values rather
+	// than counting as one operation.
 	opN int
 
 	// Writer where operations are appended to.
@@ -1064,7 +1066,7 @@ func (b *Bitmap) unmarshalPilosaRoaring(data []byte) error {
 		opr.apply(b)
 
 		// Increase the op count.
-		b.opN++
+		b.opN += opr.count()
 
 		// Move the buffer forward.
 		buf = buf[opr.size():]
@@ -1082,8 +1084,8 @@ func (b *Bitmap) writeOp(op *op) error {
 	if _, err := op.WriteTo(b.OpWriter); err != nil {
 		return err
 	}
+	b.opN += op.count()
 
-	b.opN++
 	return nil
 }
 
@@ -3646,6 +3648,18 @@ func (op *op) size() int {
 	return 1 + 8 + 4 + len(op.values)*8
 }
 
+// count returns the number of bits the operation mutates.
+func (op *op) count() int {
+	switch op.typ {
+	case 0, 1:
+		return 1
+	case 2, 3:
+		return len(op.values)
+	default:
+		panic(fmt.Sprintf("unknown operation type: %d", op.typ))
+	}
+}
+
 func highbits(v uint64) uint64 { return v >> 16 }
 func lowbits(v uint64) uint16  { return uint16(v & 0xFFFF) }
 
@@ -4118,6 +4132,7 @@ func (b *Bitmap) UnmarshalBinary(data []byte) error {
 		return nil
 	}
 	statsHit("Bitmap/UnmarshalBinary")
+	b.opN = 0 // reset opN since we're reading new data.
 	fileMagic := uint32(binary.LittleEndian.Uint16(data[0:2]))
 	if fileMagic == MagicNumber { // if pilosa roaring
 		return errors.Wrap(b.unmarshalPilosaRoaring(data), "unmarshaling as pilosa roaring")