Merge pull request #526 from SAY-5/fix/bsi-sign-bit-growth

fix(bsi): sign-extend existing negatives when bA grows in NewDefaultBSI

Author: guymolinari

roaring64/bsi64.go

@@ -100,8 +100,20 @@ func (b *BSI) SetBigValue(columnID uint64, value *big.Int) {
 		if minBits == 1 {
 			minBits = 2
 		}
-		for len(b.bA) < minBits {
-			b.bA = append(b.bA, Bitmap{})
+		if len(b.bA) < minBits {
+			oldSignPos := len(b.bA) - 1
+			for len(b.bA) < minBits {
+				b.bA = append(b.bA, Bitmap{})
+			}
+			// When bA grows, the sign slot shifts from oldSignPos to the new end
+			// of bA. For existing negative entries (whose sign bit is set in
+			// bA[oldSignPos]), sign-extension requires that all intermediate bit
+			// positions between oldSignPos and the new sign position also be set.
+			// Copy the old sign bitmap into every new slot (sign extension).
+			newSignPos := len(b.bA) - 1
+			for i := oldSignPos + 1; i <= newSignPos; i++ {
+				b.bA[i].Or(&b.bA[oldSignPos])
+			}
 		}
 	}
 
@@ -122,8 +134,16 @@ func (b *BSI) SetBigMany(foundSet *Bitmap, value *big.Int) {
 		if minBits == 1 {
 			minBits = 2
 		}
-		for len(b.bA) < minBits {
-			b.bA = append(b.bA, Bitmap{})
+		if len(b.bA) < minBits {
+			oldSignPos := len(b.bA) - 1
+			for len(b.bA) < minBits {
+				b.bA = append(b.bA, Bitmap{})
+			}
+			// Sign-extend existing negative entries into the new bit slots.
+			newSignPos := len(b.bA) - 1
+			for i := oldSignPos + 1; i <= newSignPos; i++ {
+				b.bA[i].Or(&b.bA[oldSignPos])
+			}
 		}
 	}
 	for i := b.BitCount(); i >= 0; i-- {

roaring64/bsi64_test.go

@@ -870,3 +870,26 @@ func BenchmarkClearValues(b *testing.B) {
 		b2.ClearValues(resultA)
 	}
 }
+
+// TestGetValueNegativeAfterGrowth checks that negative values stored in a
+// NewDefaultBSI are returned correctly after bA grows because a larger positive
+// value is added later. Previously, the sign bit was stranded at the old slot
+// when bA was extended, causing wrong-sign results.
+// See https://github.com/RoaringBitmap/roaring/issues/462
+func TestGetValueNegativeAfterGrowth(t *testing.T) {
+	bsi := NewDefaultBSI()
+	for i := int64(-15); i <= 15; i++ {
+		bsi.SetValue(uint64(i+15), i)
+	}
+
+	val, exists := bsi.GetValue(0)
+	require.True(t, exists)
+	assert.Equal(t, int64(-15), val, "before growth")
+
+	// Adding 16 requires an extra bit and grows bA, shifting the sign slot.
+	bsi.SetValue(uint64(16+15), 16)
+
+	val, exists = bsi.GetValue(0)
+	require.True(t, exists)
+	assert.Equal(t, int64(-15), val, "after growth")
+}