Skip to content

Commit 2cf4a7e

Browse files
committed
fix: stop assuming HybridBitmap::Large having higher cardinality than Small
Previously, when bitmap_has_all got a Small lhs and a Large rhs, it simply return false, since a set with fewer cardinality can't has_all another one with higher cardinality. But due to we don't always demote Large to Small after cardinality reducing operations, that's not true anymore. As a defense, we removed this branch, and the doubting rhs will fall into branch below, where rhs cardinality is actually checked before returning false.
1 parent 4ae69c5 commit 2cf4a7e

1 file changed

Lines changed: 2 additions & 5 deletions

File tree

src/common/io/src/bitmap.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -947,12 +947,9 @@ pub fn bitmap_has_all(lhs: &[u8], rhs: &[u8]) -> Result<bool> {
947947
return Ok(lhs.has_all_with(&rhs));
948948
}
949949

950-
// Fast path: HybridSmall lhs cannot contain HybridLarge rhs
951-
if as_roaring(lhs).is_none() && is_hybrid_large(rhs) {
952-
return Ok(false);
953-
}
954-
955950
// Fast path: HybridSmall lhs, Legacy rhs — cardinality check
951+
// We need to check cardinality here because: (1) it might be a legacy tree and (2) sometimes
952+
// we don't try demote HybridBitmap after reduce its cardinality immediately.
956953
if as_roaring(lhs).is_none() && as_roaring(rhs).is_some() {
957954
let lhs_small = SmallReader::new(lhs)?;
958955
// Fast path: rhs has more values than lhs, impossible to contain

0 commit comments

Comments
 (0)