Merge pull request #2586 from quickwit-oss/issue/2577-get_batch_multiply_overflow

follow up on the fix of multiply with overflow

Author: fulmicoton-dd

bitpacker/src/bitpacker.rs

@@ -65,7 +65,7 @@ impl BitPacker {
 
 #[derive(Clone, Debug, Default, Copy)]
 pub struct BitUnpacker {
-    num_bits: u32,
+    num_bits: usize,
     mask: u64,
 }
 
@@ -83,7 +83,7 @@ impl BitUnpacker {
             (1u64 << num_bits) - 1u64
         };
         BitUnpacker {
-            num_bits: u32::from(num_bits),
+            num_bits: usize::from(num_bits),
             mask,
         }
     }
@@ -94,7 +94,7 @@ impl BitUnpacker {
 
     #[inline]
     pub fn get(&self, idx: u32, data: &[u8]) -> u64 {
-        let addr_in_bits = idx as usize * self.num_bits as usize;
+        let addr_in_bits = idx as usize * self.num_bits;
         let addr = addr_in_bits >> 3;
         if addr + 8 > data.len() {
             if self.num_bits == 0 {
@@ -134,12 +134,13 @@ impl BitUnpacker {
             "Bitwidth must be <= 32 to use this method."
         );
 
-        let end_idx = start_idx + output.len() as u32;
+        let end_idx: u32 = start_idx + output.len() as u32;
 
-        let end_bit_read = end_idx * self.num_bits;
+        // We use `usize` here to avoid overflow issues.
+        let end_bit_read = (end_idx as usize) * self.num_bits;
         let end_byte_read = (end_bit_read + 7) / 8;
         assert!(
-            end_byte_read as usize <= data.len(),
+            end_byte_read <= data.len(),
             "Requested index is out of bounds."
         );
 
@@ -159,24 +160,24 @@ impl BitUnpacker {
         // We want the start of the fast track to start align with bytes.
         // A sufficient condition is to start with an idx that is a multiple of 8,
         // so highway start is the closest multiple of 8 that is >= start_idx.
-        let entrance_ramp_len = 8 - (start_idx % 8) % 8;
+        let entrance_ramp_len: u32 = 8 - (start_idx % 8) % 8;
 
         let highway_start: u32 = start_idx + entrance_ramp_len;
 
-        if highway_start + BitPacker1x::BLOCK_LEN as u32 > end_idx {
+        if highway_start + (BitPacker1x::BLOCK_LEN as u32) > end_idx {
             // We don't have enough values to have even a single block of highway.
             // Let's just supply the values the simple way.
             get_batch_ramp(start_idx, output);
             return;
         }
 
-        let num_blocks: u32 = (end_idx - highway_start) / BitPacker1x::BLOCK_LEN as u32;
+        let num_blocks: usize = (end_idx - highway_start) as usize / BitPacker1x::BLOCK_LEN;
 
         // Entrance ramp
         get_batch_ramp(start_idx, &mut output[..entrance_ramp_len as usize]);
 
         // Highway
-        let mut offset = (highway_start * self.num_bits) as usize / 8;
+        let mut offset = (highway_start as usize * self.num_bits) / 8;
         let mut output_cursor = (highway_start - start_idx) as usize;
         for _ in 0..num_blocks {
             offset += BitPacker1x.decompress(
@@ -188,7 +189,7 @@ impl BitUnpacker {
         }
 
         // Exit ramp
-        let highway_end = highway_start + num_blocks * BitPacker1x::BLOCK_LEN as u32;
+        let highway_end: u32 = highway_start + (num_blocks * BitPacker1x::BLOCK_LEN) as u32;
         get_batch_ramp(highway_end, &mut output[output_cursor..]);
     }
 

bitpacker/src/blocked_bitpacker.rs

@@ -34,7 +34,7 @@ struct BlockedBitpackerEntryMetaData {
 
 impl BlockedBitpackerEntryMetaData {
     fn new(offset: u64, num_bits: u8, base_value: u64) -> Self {
-        let encoded = offset | (num_bits as u64) << (64 - 8);
+        let encoded = offset | (u64::from(num_bits) << (64 - 8));
         Self {
             encoded,
             base_value,

columnar/src/tests.rs

@@ -715,6 +715,7 @@ fn test_columnar_merging_number_columns() {
 // TODO test required_columns
 // TODO document edge case: required_columns incompatible with values.
 
+#[allow(clippy::type_complexity)]
 fn columnar_docs_and_remap(
 ) -> impl Strategy<Value = (Vec<Vec<Vec<(&'static str, ColumnValue)>>>, Vec<RowAddr>)> {
     proptest::collection::vec(columnar_docs_strategy(), 2..=3).prop_flat_map(

src/indexer/index_writer.rs

@@ -2553,7 +2553,7 @@ mod tests {
     #[test]
     fn test_writer_options_validation() {
         let mut schema_builder = Schema::builder();
-        let field = schema_builder.add_bool_field("example", STORED);
+        let _field = schema_builder.add_bool_field("example", STORED);
         let index = Index::create_in_ram(schema_builder.build());
 
         let opt_wo_threads = IndexWriterOptions::builder().num_worker_threads(0).build();

src/postings/skip.rs

@@ -15,7 +15,7 @@ fn encode_bitwidth(bitwidth: u8, delta_1: bool) -> u8 {
 }
 
 fn decode_bitwidth(raw_bitwidth: u8) -> (u8, bool) {
-    let delta_1 = (raw_bitwidth >> 6 & 1) != 0;
+    let delta_1 = ((raw_bitwidth >> 6) & 1) != 0;
     let bitwidth = raw_bitwidth & 0x3f;
     (bitwidth, delta_1)
 }

src/schema/document/default_document.rs

@@ -15,7 +15,7 @@ use crate::schema::field_type::ValueParsingError;
 use crate::schema::{Facet, Field, NamedFieldDocument, OwnedValue, Schema};
 use crate::tokenizer::PreTokenizedString;
 
-#[repr(packed)]
+#[repr(C, packed)]
 #[derive(Debug, Clone)]
 /// A field value pair in the compact tantivy document
 struct FieldValueAddr {
@@ -480,7 +480,7 @@ impl<'a> CompactDocValue<'a> {
 type Addr = u32;
 
 #[derive(Clone, Copy, Default)]
-#[repr(packed)]
+#[repr(C, packed)]
 /// The value type and the address to its payload in the container.
 struct ValueAddr {
     type_id: ValueType,
@@ -734,7 +734,7 @@ mod tests {
 
     #[test]
     fn test_json_value() {
-        let json_str = r#"{ 
+        let json_str = r#"{
             "toto": "titi",
             "float": -0.2,
             "bool": true,

sstable/src/delta.rs

@@ -89,7 +89,7 @@ where
 
     fn encode_keep_add(&mut self, keep_len: usize, add_len: usize) {
         if keep_len < FOUR_BIT_LIMITS && add_len < FOUR_BIT_LIMITS {
-            let b = (keep_len | add_len << 4) as u8;
+            let b = (keep_len | (add_len << 4)) as u8;
             self.block.extend_from_slice(&[b])
         } else {
             let mut buf = [VINT_MODE; 20];

sstable/src/dictionary.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::needless_borrows_for_generic_args)]
+
 use std::cmp::Ordering;
 use std::io;
 use std::marker::PhantomData;

stacker/src/memory_arena.rs

@@ -54,7 +54,7 @@ impl Addr {
 
     #[inline]
     fn new(page_id: usize, local_addr: usize) -> Addr {
-        Addr((page_id << NUM_BITS_PAGE_ADDR | local_addr) as u32)
+        Addr(((page_id << NUM_BITS_PAGE_ADDR) | local_addr) as u32)
     }
 
     #[inline]