Merge pull request #238 from RoaringBitmap/bug-optimize-select

Fix for select bug in runcontainer

optimize_test.go

@@ -0,0 +1,49 @@
+package roaring_test
+
+import (
+	"github.com/RoaringBitmap/roaring"
+	"testing"
+)
+
+func TestSelectAfterOptimize(t *testing.T) {
+	r := roaring.New()
+	intArray := []uint32{438945, 438946, 438947, 438948, 438949, 438950, 438951, 438952, 438953, 438954, 438955, 438956, 438957, 438958, 438959, 438960, 438961, 438962, 438963, 438964, 438965, 438966, 438967, 438968, 438969, 438970, 438971, 438972, 438973, 438974, 438975, 438976, 438977, 438978, 438979, 438980, 438981, 438982, 438983, 438984, 438985, 438986, 438987, 438988, 438989, 438990, 438991, 438992, 438993, 438994, 438995, 438996, 438997, 438998, 438999, 439000, 439001, 439002, 439003, 439004, 439005, 439006, 439007, 439008, 439009, 439010, 439011, 439012, 439013, 439014, 439015, 439016, 439017, 439018, 439019, 439020, 439021, 439022, 439023, 439024, 439025, 439026, 439027, 439028, 439029, 439030, 439031, 439032, 439033, 439034, 439035, 439036, 439037, 439038, 439039, 439040, 439041, 439042, 439043, 439044, 439045, 439046, 439047, 439048, 439049, 439050, 439051, 439052, 439053, 439054, 439055, 439056, 439057, 439058, 439059, 439060, 439061, 439062, 439063, 439064, 439065, 439066, 439067, 439068, 439069, 439070, 439071, 439072, 439073, 439074, 439075, 439076, 439077, 439078, 439079, 439080, 439081, 439082, 439083, 439084, 439085, 439086, 439087, 439088, 439089, 439090, 439091, 439092, 439093, 439094, 439095, 439096, 439097, 439098, 439099, 439100, 439101, 439102, 439103, 439104, 439105, 439106, 439107, 439108, 439109, 439110, 439111, 439112, 439113, 439114, 439115, 439116, 439117, 439118, 439119, 439120, 439121, 439122, 439123, 439124, 439125, 439126, 439127, 439128, 439129, 439130, 439131, 439132, 439133, 439134, 439135, 439136, 439137, 439138, 439139, 439140, 439141, 439142, 439143, 439144, 439145, 439146, 439147, 439148, 439149, 439150, 439151, 439152, 439153, 439154, 439155, 439156, 439157, 439158, 439159, 439160, 439161, 439162, 439163, 439164, 439165, 439166, 439167, 439168, 439169, 439170, 439171, 439172, 439173, 439174, 439175, 439176, 439177, 439178, 439179, 439180, 439181, 439182, 439183, 439184, 439185, 439186, 439187, 439188, 439189, 439190, 439191, 439192, 439193, 439194, 439195, 439196, 439197, 439198, 439199, 439200, 439201, 439202, 439203, 439204, 439205, 439206, 439207, 439208, 439209, 439210, 439211, 439212, 439213, 439214, 439215, 439216, 439217, 439218, 439219, 439220, 439221, 439222, 439223, 439224, 439225, 439226, 439227, 439228, 439229, 439230, 439231, 439232, 439233, 439234, 439235, 439236, 439237, 439238, 439239, 439240, 439241, 439242, 439243, 439244, 439245, 439246, 439248}
+	for _, value := range intArray {
+		r.Add(value)
+	}
+
+	// save original version as array
+	origArray := r.ToArray()
+
+	// comment this out to get a passing test
+	r.RunOptimize()
+
+	// get a list of values after optimize
+	optimized := r.ToArray()
+
+	// this should be fine in both cases
+	if diff := len(optimized) - len(origArray); diff != 0 {
+		t.Fatal("element count different - diff:", diff)
+	}
+
+	// this is also fine
+	for i := range optimized {
+		if optimized[i] != origArray[i] {
+			t.Errorf("array compare %d", i)
+		}
+	}
+
+	// this produces errors with the optimized version of the bitmap
+	n := r.GetCardinality()
+	for i := uint64(0); i < n; i++ {
+
+		v, err := r.Select(uint32(i))
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if diff := origArray[i] - v; diff != 0 {
+			t.Errorf("select %03d failed - %d vs %d (diff:%d)", i, origArray[i], v, diff)
+		}
+	}
+}

runcontainer.go

@@ -1442,7 +1442,7 @@ func (rc *runContainer16) selectInt16(j uint16) int {
 
 	var offset int64
 	for k := range rc.iv {
-		nextOffset := offset + rc.iv[k].runlen() + 1
+		nextOffset := offset + rc.iv[k].runlen()
 		if nextOffset > int64(j) {
 			return int(int64(rc.iv[k].start) + (int64(j) - offset))
 		}