Skip to content

Commit 8b650a6

Browse files
authored
Merge pull request #330 from Dr-Emann/fix_vector_array_sub
fix: do not accidently remove zeros in vector sub
2 parents 5365315 + cdb1d53 commit 8b650a6

File tree

2 files changed

+31
-5
lines changed

2 files changed

+31
-5
lines changed

roaring/src/bitmap/store/array_store/vector.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -331,11 +331,20 @@ pub fn sub(lhs: &[u16], rhs: &[u16], visitor: &mut impl BinaryOperationVisitor)
331331
// or i_b == st_b and we are not done processing the vector...
332332
// so we need to finish it off.
333333
if i < st_a {
334-
let mut buffer: [u16; 8] = [0; 8]; // buffer to do a masked load
335-
buffer[..rhs.len() - j].copy_from_slice(&rhs[j..]);
336-
v_b = Simd::from_array(buffer);
337-
let a_found_in_b: u8 = matrix_cmp_u16(v_a, v_b).to_bitmask() as u8;
338-
runningmask_a_found_in_b |= a_found_in_b;
334+
let remaining_rhs = &rhs[j..];
335+
if !remaining_rhs.is_empty() {
336+
let mut buffer: [u16; 8] = [0; 8]; // buffer to do a masked load
337+
buffer[..remaining_rhs.len()].copy_from_slice(remaining_rhs);
338+
// Ensure the buffer is filled with a value we should remove: we do not want to
339+
// end up trying to remove zero values which aren't actually in rhs
340+
buffer[remaining_rhs.len()..].fill(remaining_rhs[0]);
341+
v_b = Simd::from_array(buffer);
342+
let a_found_in_b: u8 = matrix_cmp_u16(v_a, v_b).to_bitmask() as u8;
343+
runningmask_a_found_in_b |= a_found_in_b;
344+
let [.., max_va] = *v_a.as_array();
345+
let used_rhs = remaining_rhs.partition_point(|&b| b <= max_va);
346+
j += used_rhs;
347+
}
339348
let bitmask_belongs_to_difference: u8 = runningmask_a_found_in_b ^ 0xFF;
340349
visitor.visit_vector(v_a, bitmask_belongs_to_difference);
341350
i += u16x8::LEN;

roaring/tests/ops.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,23 @@ fn sub() {
5555
assert_eq!(rb3, rb1);
5656
}
5757

58+
// See issue #327
59+
#[test]
60+
fn subtraction_preserves_zero_element() {
61+
let mut a = RoaringBitmap::from([0, 35, 80, 104, 138, 214, 235, 258]);
62+
let b = RoaringBitmap::from([9, 35, 42, 51, 111, 134, 231, 239]);
63+
64+
a -= b;
65+
66+
// The bug: element 0 should still be present but was being removed
67+
assert!(a.contains(0), "Element 0 should be present after subtraction");
68+
69+
// Verify the complete result
70+
let expected: Vec<u32> = vec![0, 80, 104, 138, 214, 235, 258];
71+
let actual: Vec<u32> = a.iter().collect();
72+
assert_eq!(actual, expected, "Subtraction result should match expected values");
73+
}
74+
5875
#[test]
5976
fn xor() {
6077
let mut rb1 = (1..4).collect::<RoaringBitmap>();

0 commit comments

Comments
 (0)