clarify tokenizer docs

columnar/src/column_index/merge/mod.rs

@@ -1,73 +1,19 @@
 mod shuffled;
 mod stacked;
 
-use common::ReadOnlyBitSet;
 use shuffled::merge_column_index_shuffled;
 use stacked::merge_column_index_stacked;
 
 use crate::column_index::SerializableColumnIndex;
 use crate::{Cardinality, ColumnIndex, MergeRowOrder};
 
-fn detect_cardinality_single_column_index(
-    column_index: &ColumnIndex,
-    alive_bitset_opt: &Option<ReadOnlyBitSet>,
-) -> Cardinality {
-    let Some(alive_bitset) = alive_bitset_opt else {
-        return column_index.get_cardinality();
-    };
-    let cardinality_before_deletes = column_index.get_cardinality();
-    if cardinality_before_deletes == Cardinality::Full {
-        // The columnar cardinality can only become more restrictive in the presence of deletes
-        // (where cardinality sorted from the more restrictive to the least restrictive are Full,
-        // Optional, Multivalued)
-        //
-        // If we are already "Full", we are guaranteed to stay "Full" after deletes.
-        return Cardinality::Full;
-    }
-    let mut cardinality_so_far = Cardinality::Full;
-    for doc_id in alive_bitset.iter() {
-        let num_values = column_index.value_row_ids(doc_id).len();
-        let row_cardinality = match num_values {
-            0 => Cardinality::Optional,
-            1 => Cardinality::Full,
-            _ => Cardinality::Multivalued,
-        };
-        cardinality_so_far = cardinality_so_far.max(row_cardinality);
-        if cardinality_so_far >= cardinality_before_deletes {
-            // There won't be any improvement in the cardinality.
-            // We can early exit.
-            return cardinality_before_deletes;
-        }
-    }
-    cardinality_so_far
-}
-
-fn detect_cardinality(
-    column_indexes: &[ColumnIndex],
-    merge_row_order: &MergeRowOrder,
-) -> Cardinality {
-    match merge_row_order {
-        MergeRowOrder::Stack(_) => column_indexes
-            .iter()
-            .map(ColumnIndex::get_cardinality)
-            .max()
-            .unwrap_or(Cardinality::Full),
-        MergeRowOrder::Shuffled(shuffle_merge_order) => {
-            let mut merged_cardinality = Cardinality::Full;
-            for (column_index, alive_bitset_opt) in column_indexes
-                .iter()
-                .zip(shuffle_merge_order.alive_bitsets.iter())
-            {
-                let cardinality: Cardinality =
-                    detect_cardinality_single_column_index(column_index, alive_bitset_opt);
-                if cardinality == Cardinality::Multivalued {
-                    return cardinality;
-                }
-                merged_cardinality = merged_cardinality.max(cardinality);
-            }
-            merged_cardinality
-        }
-    }
+// For simplification, we never have cardinality go down due to deletes.
+fn detect_cardinality(columns: &[ColumnIndex]) -> Cardinality {
+    columns
+        .iter()
+        .map(ColumnIndex::get_cardinality)
+        .max()
+        .unwrap_or(Cardinality::Full)
 }
 
 pub fn merge_column_index<'a>(
@@ -76,7 +22,7 @@ pub fn merge_column_index<'a>(
 ) -> SerializableColumnIndex<'a> {
     // For simplification, we do not try to detect whether the cardinality could be
     // downgraded thanks to deletes.
-    let cardinality_after_merge = detect_cardinality(columns, merge_row_order);
+    let cardinality_after_merge = detect_cardinality(columns);
     match merge_row_order {
         MergeRowOrder::Stack(stack_merge_order) => {
             merge_column_index_stacked(columns, cardinality_after_merge, stack_merge_order)
@@ -98,54 +44,34 @@ mod tests {
     use crate::column_index::merge::detect_cardinality;
     use crate::column_index::multivalued_index::MultiValueIndex;
     use crate::column_index::{merge_column_index, OptionalIndex, SerializableColumnIndex};
-    use crate::{
-        Cardinality, ColumnIndex, MergeRowOrder, RowAddr, RowId, ShuffleMergeOrder, StackMergeOrder,
-    };
+    use crate::{Cardinality, ColumnIndex, MergeRowOrder, RowAddr, RowId, ShuffleMergeOrder};
 
     #[test]
     fn test_detect_cardinality() {
-        assert_eq!(
-            detect_cardinality(&[], &StackMergeOrder::stack_for_test(&[]).into()),
-            Cardinality::Full
-        );
+        assert_eq!(detect_cardinality(&[]), Cardinality::Full);
         let optional_index: ColumnIndex = OptionalIndex::for_test(1, &[]).into();
         let multivalued_index: ColumnIndex = MultiValueIndex::for_test(&[0, 1]).into();
         assert_eq!(
-            detect_cardinality(
-                &[optional_index.clone(), ColumnIndex::Empty { num_docs: 0 }],
-                &StackMergeOrder::stack_for_test(&[1, 0]).into()
-            ),
+            detect_cardinality(&[optional_index.clone(), ColumnIndex::Empty { num_docs: 0 }]),
             Cardinality::Optional
         );
         assert_eq!(
-            detect_cardinality(
-                &[optional_index.clone(), ColumnIndex::Full],
-                &StackMergeOrder::stack_for_test(&[1, 1]).into()
-            ),
+            detect_cardinality(&[optional_index.clone(), ColumnIndex::Full]),
             Cardinality::Optional
         );
         assert_eq!(
-            detect_cardinality(
-                &[
-                    multivalued_index.clone(),
-                    ColumnIndex::Empty { num_docs: 0 }
-                ],
-                &StackMergeOrder::stack_for_test(&[1, 0]).into()
-            ),
+            detect_cardinality(&[
+                multivalued_index.clone(),
+                ColumnIndex::Empty { num_docs: 0 }
+            ]),
             Cardinality::Multivalued
         );
         assert_eq!(
-            detect_cardinality(
-                &[multivalued_index.clone(), optional_index.clone()],
-                &StackMergeOrder::stack_for_test(&[1, 1]).into()
-            ),
+            detect_cardinality(&[multivalued_index.clone(), optional_index.clone()]),
             Cardinality::Multivalued
         );
         assert_eq!(
-            detect_cardinality(
-                &[optional_index, multivalued_index],
-                &StackMergeOrder::stack_for_test(&[1, 1]).into()
-            ),
+            detect_cardinality(&[optional_index, multivalued_index]),
             Cardinality::Multivalued
         );
     }

columnar/src/columnar/merge/merge_dict_column.rs

@@ -1,7 +1,7 @@
 use std::io::{self, Write};
 
 use common::{BitSet, CountingWriter, ReadOnlyBitSet};
-use sstable::{SSTable, Streamer, TermOrdinal, VoidSSTable};
+use sstable::{SSTable, TermOrdinal};
 
 use super::term_merger::TermMerger;
 use crate::column::serialize_column_mappable_to_u64;
@@ -52,23 +52,18 @@ impl<'a> Iterable for RemappedTermOrdinalsValues<'a> {
 
 impl<'a> RemappedTermOrdinalsValues<'a> {
     fn boxed_iter_stacked(&self) -> Box<dyn Iterator<Item = u64> + '_> {
-        let iter = self
-            .bytes_columns
-            .iter()
-            .enumerate()
-            .flat_map(|(seg_ord, bytes_column_opt)| {
-                let bytes_column = bytes_column_opt.as_ref()?;
-                Some((seg_ord, bytes_column))
-            })
-            .flat_map(move |(seg_ord, bytes_column)| {
-                let term_ord_after_merge_mapping =
-                    self.term_ord_mapping.get_segment(seg_ord as u32);
+        let iter = self.bytes_columns.iter().flatten().enumerate().flat_map(
+            move |(seg_ord_with_column, bytes_column)| {
+                let term_ord_after_merge_mapping = self
+                    .term_ord_mapping
+                    .get_segment(seg_ord_with_column as u32);
                 bytes_column
                     .ords()
                     .values
                     .iter()
                     .map(move |term_ord| term_ord_after_merge_mapping[term_ord as usize])
-            });
+            },
+        );
         Box::new(iter)
     }
 
@@ -126,15 +121,10 @@ fn serialize_merged_dict(
     let mut term_ord_mapping = TermOrdinalMapping::default();
 
     let mut field_term_streams = Vec::new();
-    for column_opt in bytes_columns.iter() {
-        if let Some(column) = column_opt {
-            term_ord_mapping.add_segment(column.dictionary.num_terms());
-            let terms: Streamer<VoidSSTable> = column.dictionary.stream()?;
-            field_term_streams.push(terms);
-        } else {
-            term_ord_mapping.add_segment(0);
-            field_term_streams.push(Streamer::empty());
-        }
+    for column in bytes_columns.iter().flatten() {
+        term_ord_mapping.add_segment(column.dictionary.num_terms());
+        let terms = column.dictionary.stream()?;
+        field_term_streams.push(terms);
     }
 
     let mut merged_terms = TermMerger::new(field_term_streams);

columnar/src/columnar/merge/merge_mapping.rs

@@ -11,17 +11,6 @@ pub struct StackMergeOrder {
 }
 
 impl StackMergeOrder {
-    #[cfg(test)]
-    pub fn stack_for_test(num_rows_per_columnar: &[u32]) -> StackMergeOrder {
-        let mut cumulated_row_ids: Vec<RowId> = Vec::with_capacity(num_rows_per_columnar.len());
-        let mut cumulated_row_id = 0;
-        for &num_rows in num_rows_per_columnar {
-            cumulated_row_id += num_rows;
-            cumulated_row_ids.push(cumulated_row_id);
-        }
-        StackMergeOrder { cumulated_row_ids }
-    }
-
     pub fn stack(columnars: &[&ColumnarReader]) -> StackMergeOrder {
         let mut cumulated_row_ids: Vec<RowId> = Vec::with_capacity(columnars.len());
         let mut cumulated_row_id = 0;

columnar/src/columnar/merge/mod.rs

@@ -7,7 +7,6 @@ use std::io;
 use std::net::Ipv6Addr;
 use std::sync::Arc;
 
-use itertools::Itertools;
 pub use merge_mapping::{MergeRowOrder, ShuffleMergeOrder, StackMergeOrder};
 
 use super::writer::ColumnarSerializer;
@@ -83,11 +82,10 @@ pub fn merge_columnar(
         .iter()
         .map(|reader| reader.num_rows())
         .collect::<Vec<u32>>();
-    let columns_to_merge =
-        group_columns_for_merge(columnar_readers, required_columns, &merge_row_order)?;
+    let columns_to_merge = group_columns_for_merge(columnar_readers, required_columns)?;
     for ((column_name, column_type), columns) in columns_to_merge {
         let mut column_serializer =
-            serializer.start_serialize_column(column_name.as_bytes(), column_type);
+            serializer.serialize_column(column_name.as_bytes(), column_type);
         merge_column(
             column_type,
             &num_rows_per_columnar,
@@ -95,7 +93,6 @@ pub fn merge_columnar(
             &merge_row_order,
             &mut column_serializer,
         )?;
-        column_serializer.finalize()?;
     }
     serializer.finalize(merge_row_order.num_rows())?;
     Ok(())
@@ -290,69 +287,10 @@ fn merged_numerical_columns_type<'a>(
     compatible_numerical_types.to_numerical_type()
 }
 
-fn is_empty_after_merge(
-    merge_row_order: &MergeRowOrder,
-    column: &DynamicColumn,
-    columnar_id: usize,
-) -> bool {
-    if column.num_values() == 0u32 {
-        // It was empty before the merge.
-        return true;
-    }
-    match merge_row_order {
-        MergeRowOrder::Stack(_) => {
-            // If we are stacking the columnar, no rows are being deleted.
-            false
-        }
-        MergeRowOrder::Shuffled(shuffled) => {
-            if let Some(alive_bitset) = &shuffled.alive_bitsets[columnar_id] {
-                let column_index = column.column_index();
-                match column_index {
-                    ColumnIndex::Empty { .. } => true,
-                    ColumnIndex::Full => alive_bitset.len() == 0,
-                    ColumnIndex::Optional(optional_index) => {
-                        for doc in optional_index.iter_rows() {
-                            if alive_bitset.contains(doc) {
-                                return false;
-                            }
-                        }
-                        true
-                    }
-                    ColumnIndex::Multivalued(multivalued_index) => {
-                        for (doc_id, (start_index, end_index)) in multivalued_index
-                            .start_index_column
-                            .iter()
-                            .tuple_windows()
-                            .enumerate()
-                        {
-                            let doc_id = doc_id as u32;
-                            if start_index == end_index {
-                                // There are no values in this document
-                                continue;
-                            }
-                            // The document contains values and is present in the alive bitset.
-                            // The column is therefore not empty.
-                            if alive_bitset.contains(doc_id) {
-                                return false;
-                            }
-                        }
-                        true
-                    }
-                }
-            } else {
-                // No document is being deleted.
-                // The shuffle is applying a permutation.
-                false
-            }
-        }
-    }
-}
-
 #[allow(clippy::type_complexity)]
 fn group_columns_for_merge(
     columnar_readers: &[&ColumnarReader],
     required_columns: &[(String, ColumnType)],
-    merge_row_order: &MergeRowOrder,
 ) -> io::Result<BTreeMap<(String, ColumnType), Vec<Option<DynamicColumn>>>> {
     // Each column name may have multiple types of column associated.
     // For merging we are interested in the same column type category since they can be merged.
@@ -369,16 +307,9 @@ fn group_columns_for_merge(
 
     for (columnar_id, columnar_reader) in columnar_readers.iter().enumerate() {
         let column_name_and_handle = columnar_reader.list_columns()?;
-        // We skip columns that end up with 0 documents.
-        // That way, we make sure they don't end up influencing the merge type or
-        // creating empty columns.
-
         for (column_name, handle) in column_name_and_handle {
             let column_category: ColumnTypeCategory = handle.column_type().into();
             let column = handle.open()?;
-            if is_empty_after_merge(merge_row_order, &column, columnar_id) {
-                continue;
-            }
             columns_grouped
                 .entry((column_name, column_category))
                 .or_insert_with(|| {

columnar/src/columnar/merge/tests.rs

@@ -25,10 +25,8 @@ fn test_column_coercion_to_u64() {
     let columnar1 = make_columnar("numbers", &[1i64]);
     // u64 type
     let columnar2 = make_columnar("numbers", &[u64::MAX]);
-    let columnars = &[&columnar1, &columnar2];
-    let merge_order = StackMergeOrder::stack(columnars).into();
     let column_map: BTreeMap<(String, ColumnType), Vec<Option<DynamicColumn>>> =
-        group_columns_for_merge(columnars, &[], &merge_order).unwrap();
+        group_columns_for_merge(&[&columnar1, &columnar2], &[]).unwrap();
     assert_eq!(column_map.len(), 1);
     assert!(column_map.contains_key(&("numbers".to_string(), ColumnType::U64)));
 }
@@ -37,10 +35,8 @@ fn test_column_coercion_to_u64() {
 fn test_column_no_coercion_if_all_the_same() {
     let columnar1 = make_columnar("numbers", &[1u64]);
     let columnar2 = make_columnar("numbers", &[2u64]);
-    let columnars = &[&columnar1, &columnar2];
-    let merge_order = StackMergeOrder::stack(columnars).into();
     let column_map: BTreeMap<(String, ColumnType), Vec<Option<DynamicColumn>>> =
-        group_columns_for_merge(columnars, &[], &merge_order).unwrap();
+        group_columns_for_merge(&[&columnar1, &columnar2], &[]).unwrap();
     assert_eq!(column_map.len(), 1);
     assert!(column_map.contains_key(&("numbers".to_string(), ColumnType::U64)));
 }
@@ -49,10 +45,8 @@ fn test_column_no_coercion_if_all_the_same() {
 fn test_column_coercion_to_i64() {
     let columnar1 = make_columnar("numbers", &[-1i64]);
     let columnar2 = make_columnar("numbers", &[2u64]);
-    let columnars = &[&columnar1, &columnar2];
-    let merge_order = StackMergeOrder::stack(columnars).into();
     let column_map: BTreeMap<(String, ColumnType), Vec<Option<DynamicColumn>>> =
-        group_columns_for_merge(columnars, &[], &merge_order).unwrap();
+        group_columns_for_merge(&[&columnar1, &columnar2], &[]).unwrap();
     assert_eq!(column_map.len(), 1);
     assert!(column_map.contains_key(&("numbers".to_string(), ColumnType::I64)));
 }
@@ -60,13 +54,10 @@ fn test_column_coercion_to_i64() {
 #[test]
 fn test_impossible_coercion_returns_an_error() {
     let columnar1 = make_columnar("numbers", &[u64::MAX]);
-    let merge_order = StackMergeOrder::stack(&[&columnar1]).into();
-    let group_error = group_columns_for_merge(
-        &[&columnar1],
-        &[("numbers".to_string(), ColumnType::I64)],
-        &merge_order,
-    )
-    .unwrap_err();
+    let group_error =
+        group_columns_for_merge(&[&columnar1], &[("numbers".to_string(), ColumnType::I64)])
+            .map(|_| ())
+            .unwrap_err();
     assert_eq!(group_error.kind(), io::ErrorKind::InvalidInput);
 }
 
@@ -74,13 +65,10 @@ fn test_impossible_coercion_returns_an_error() {
 fn test_group_columns_with_required_column() {
     let columnar1 = make_columnar("numbers", &[1i64]);
     let columnar2 = make_columnar("numbers", &[2u64]);
-    let columnars = &[&columnar1, &columnar2];
-    let merge_order = StackMergeOrder::stack(columnars).into();
     let column_map: BTreeMap<(String, ColumnType), Vec<Option<DynamicColumn>>> =
         group_columns_for_merge(
             &[&columnar1, &columnar2],
             &[("numbers".to_string(), ColumnType::U64)],
-            &merge_order,
         )
         .unwrap();
     assert_eq!(column_map.len(), 1);
@@ -91,13 +79,10 @@ fn test_group_columns_with_required_column() {
 fn test_group_columns_required_column_with_no_existing_columns() {
     let columnar1 = make_columnar("numbers", &[2u64]);
     let columnar2 = make_columnar("numbers", &[2u64]);
-    let columnars = &[&columnar1, &columnar2];
-    let merge_order = StackMergeOrder::stack(columnars).into();
     let column_map: BTreeMap<(String, ColumnType), Vec<Option<DynamicColumn>>> =
         group_columns_for_merge(
-            columnars,
+            &[&columnar1, &columnar2],
             &[("required_col".to_string(), ColumnType::Str)],
-            &merge_order,
         )
         .unwrap();
     assert_eq!(column_map.len(), 2);
@@ -113,13 +98,10 @@ fn test_group_columns_required_column_with_no_existing_columns() {
 fn test_group_columns_required_column_is_above_all_columns_have_the_same_type_rule() {
     let columnar1 = make_columnar("numbers", &[2i64]);
     let columnar2 = make_columnar("numbers", &[2i64]);
-    let columnars = &[&columnar1, &columnar2];
-    let merge_order = StackMergeOrder::stack(columnars).into();
     let column_map: BTreeMap<(String, ColumnType), Vec<Option<DynamicColumn>>> =
         group_columns_for_merge(
-            columnars,
+            &[&columnar1, &columnar2],
             &[("numbers".to_string(), ColumnType::U64)],
-            &merge_order,
         )
         .unwrap();
     assert_eq!(column_map.len(), 1);
@@ -130,10 +112,8 @@ fn test_group_columns_required_column_is_above_all_columns_have_the_same_type_ru
 fn test_missing_column() {
     let columnar1 = make_columnar("numbers", &[-1i64]);
     let columnar2 = make_columnar("numbers2", &[2u64]);
-    let columnars = &[&columnar1, &columnar2];
-    let merge_order = StackMergeOrder::stack(columnars).into();
     let column_map: BTreeMap<(String, ColumnType), Vec<Option<DynamicColumn>>> =
-        group_columns_for_merge(columnars, &[], &merge_order).unwrap();
+        group_columns_for_merge(&[&columnar1, &columnar2], &[]).unwrap();
     assert_eq!(column_map.len(), 2);
     assert!(column_map.contains_key(&("numbers".to_string(), ColumnType::I64)));
     {

columnar/src/columnar/writer/mod.rs

@@ -370,7 +370,7 @@ impl ColumnarWriter {
                     let column_writer: ColumnWriter = self.bool_field_hash_map.read(addr);
                     let cardinality = column_writer.get_cardinality(num_docs);
                     let mut column_serializer =
-                        serializer.start_serialize_column(column_name, column_type);
+                        serializer.serialize_column(column_name, column_type);
                     serialize_bool_column(
                         cardinality,
                         num_docs,
@@ -382,13 +382,12 @@ impl ColumnarWriter {
                         buffers,
                         &mut column_serializer,
                     )?;
-                    column_serializer.finalize()?;
                 }
                 ColumnType::IpAddr => {
                     let column_writer: ColumnWriter = self.ip_addr_field_hash_map.read(addr);
                     let cardinality = column_writer.get_cardinality(num_docs);
                     let mut column_serializer =
-                        serializer.start_serialize_column(column_name, ColumnType::IpAddr);
+                        serializer.serialize_column(column_name, ColumnType::IpAddr);
                     serialize_ip_addr_column(
                         cardinality,
                         num_docs,
@@ -400,7 +399,6 @@ impl ColumnarWriter {
                         buffers,
                         &mut column_serializer,
                     )?;
-                    column_serializer.finalize()?;
                 }
                 ColumnType::Bytes | ColumnType::Str => {
                     let str_or_bytes_column_writer: StrOrBytesColumnWriter =
@@ -415,7 +413,7 @@ impl ColumnarWriter {
                         .column_writer
                         .get_cardinality(num_docs);
                     let mut column_serializer =
-                        serializer.start_serialize_column(column_name, column_type);
+                        serializer.serialize_column(column_name, column_type);
                     serialize_bytes_or_str_column(
                         cardinality,
                         num_docs,
@@ -429,14 +427,13 @@ impl ColumnarWriter {
                         buffers,
                         &mut column_serializer,
                     )?;
-                    column_serializer.finalize()?;
                 }
                 ColumnType::F64 | ColumnType::I64 | ColumnType::U64 => {
                     let numerical_column_writer: NumericalColumnWriter =
                         self.numerical_field_hash_map.read(addr);
                     let cardinality = numerical_column_writer.cardinality(num_docs);
                     let mut column_serializer =
-                        serializer.start_serialize_column(column_name, column_type);
+                        serializer.serialize_column(column_name, column_type);
                     let numerical_type = column_type.numerical_type().unwrap();
                     serialize_numerical_column(
                         cardinality,
@@ -450,13 +447,12 @@ impl ColumnarWriter {
                         buffers,
                         &mut column_serializer,
                     )?;
-                    column_serializer.finalize()?;
                 }
                 ColumnType::DateTime => {
                     let column_writer: ColumnWriter = self.datetime_field_hash_map.read(addr);
                     let cardinality = column_writer.get_cardinality(num_docs);
                     let mut column_serializer =
-                        serializer.start_serialize_column(column_name, ColumnType::DateTime);
+                        serializer.serialize_column(column_name, ColumnType::DateTime);
                     serialize_numerical_column(
                         cardinality,
                         num_docs,
@@ -469,7 +465,6 @@ impl ColumnarWriter {
                         buffers,
                         &mut column_serializer,
                     )?;
-                    column_serializer.finalize()?;
                 }
             };
         }

columnar/src/columnar/writer/serializer.rs

@@ -34,12 +34,11 @@ impl<W: io::Write> ColumnarSerializer<W> {
         }
     }
 
-    /// Creates a ColumnSerializer.
-    pub fn start_serialize_column<'a>(
+    pub fn serialize_column<'a>(
         &'a mut self,
         column_name: &[u8],
         column_type: ColumnType,
-    ) -> ColumnSerializer<'a, W> {
+    ) -> impl io::Write + 'a {
         let start_offset = self.wrt.written_bytes();
         prepare_key(column_name, column_type, &mut self.prepare_key_buffer);
         ColumnSerializer {
@@ -61,21 +60,20 @@ impl<W: io::Write> ColumnarSerializer<W> {
     }
 }
 
-pub struct ColumnSerializer<'a, W: io::Write> {
+struct ColumnSerializer<'a, W: io::Write> {
     columnar_serializer: &'a mut ColumnarSerializer<W>,
     start_offset: u64,
 }
 
-impl<'a, W: io::Write> ColumnSerializer<'a, W> {
-    pub fn finalize(self) -> io::Result<()> {
+impl<'a, W: io::Write> Drop for ColumnSerializer<'a, W> {
+    fn drop(&mut self) {
         let end_offset: u64 = self.columnar_serializer.wrt.written_bytes();
         let byte_range = self.start_offset..end_offset;
-        self.columnar_serializer.sstable_range.insert(
+        self.columnar_serializer.sstable_range.insert_cannot_fail(
             &self.columnar_serializer.prepare_key_buffer[..],
             &byte_range,
-        )?;
+        );
         self.columnar_serializer.prepare_key_buffer.clear();
-        Ok(())
     }
 }
 

columnar/src/lib.rs

@@ -39,7 +39,7 @@ pub use self::dynamic_column::{DynamicColumn, DynamicColumnHandle};
 pub type RowId = u32;
 pub type DocId = u32;
 
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy)]
 pub struct RowAddr {
     pub segment_ord: u32,
     pub row_id: RowId,

columnar/src/tests.rs

@@ -4,15 +4,13 @@ use std::net::Ipv6Addr;
 
 use common::DateTime;
 use proptest::prelude::*;
-use proptest::sample::subsequence;
 
 use crate::column_values::MonotonicallyMappableToU128;
 use crate::columnar::{ColumnType, ColumnTypeCategory};
 use crate::dynamic_column::{DynamicColumn, DynamicColumnHandle};
 use crate::value::{Coerce, NumericalValue};
 use crate::{
-    BytesColumn, Cardinality, Column, ColumnarReader, ColumnarWriter, RowAddr, RowId,
-    ShuffleMergeOrder, StackMergeOrder,
+    BytesColumn, Cardinality, Column, ColumnarReader, ColumnarWriter, RowId, StackMergeOrder,
 };
 
 #[test]
@@ -262,15 +260,12 @@ fn test_dictionary_encoded_bytes() {
 
 fn num_strategy() -> impl Strategy<Value = NumericalValue> {
     prop_oneof![
-        3 => Just(NumericalValue::U64(0u64)),
-        3 => Just(NumericalValue::U64(u64::MAX)),
-        3 => Just(NumericalValue::I64(0i64)),
-        3 => Just(NumericalValue::I64(i64::MIN)),
-        3 => Just(NumericalValue::I64(i64::MAX)),
-        3 => Just(NumericalValue::F64(1.2f64)),
-        1 => any::<f64>().prop_map(NumericalValue::from),
-        1 => any::<u64>().prop_map(NumericalValue::from),
-        1 => any::<i64>().prop_map(NumericalValue::from),
+        Just(NumericalValue::U64(0u64)),
+        Just(NumericalValue::U64(u64::MAX)),
+        Just(NumericalValue::I64(0i64)),
+        Just(NumericalValue::I64(i64::MIN)),
+        Just(NumericalValue::I64(i64::MAX)),
+        Just(NumericalValue::F64(1.2f64)),
     ]
 }
 
@@ -284,12 +279,6 @@ enum ColumnValue {
     DateTime(DateTime),
 }
 
-impl<T: Into<NumericalValue>> From<T> for ColumnValue {
-    fn from(val: T) -> ColumnValue {
-        ColumnValue::Numerical(val.into())
-    }
-}
-
 impl ColumnValue {
     pub(crate) fn column_type_category(&self) -> ColumnTypeCategory {
         match self {
@@ -339,22 +328,12 @@ fn column_value_strategy() -> impl Strategy<Value = ColumnValue> {
 
 // A document contains up to 4 values.
 fn doc_strategy() -> impl Strategy<Value = Vec<(&'static str, ColumnValue)>> {
-    proptest::collection::vec((column_name_strategy(), column_value_strategy()), 0..=4)
-}
-
-fn num_docs_strategy() -> impl Strategy<Value = usize> {
-    prop_oneof!(
-        // We focus heavily on the 0..2 case as we assume it is sufficient to cover all edge cases.
-        0usize..=3usize,
-        // We leave 50% of the effort exploring more defensively.
-        3usize..=12usize
-    )
+    proptest::collection::vec((column_name_strategy(), column_value_strategy()), 0..4)
 }
 
 // A columnar contains up to 2 docs.
 fn columnar_docs_strategy() -> impl Strategy<Value = Vec<Vec<(&'static str, ColumnValue)>>> {
-    num_docs_strategy()
-        .prop_flat_map(|num_docs| proptest::collection::vec(doc_strategy(), num_docs))
+    proptest::collection::vec(doc_strategy(), 0..=2)
 }
 
 fn columnar_docs_and_mapping_strategy(
@@ -368,11 +347,6 @@ fn permutation_strategy(n: usize) -> impl Strategy<Value = Vec<RowId>> {
     Just((0u32..n as RowId).collect()).prop_shuffle()
 }
 
-fn permutation_and_subset_strategy(n: usize) -> impl Strategy<Value = Vec<usize>> {
-    let vals: Vec<usize> = (0..n).collect();
-    subsequence(vals, 0..=n).prop_shuffle()
-}
-
 fn build_columnar_with_mapping(
     docs: &[Vec<(&'static str, ColumnValue)>],
     old_to_new_row_ids_opt: Option<&[RowId]>,
@@ -415,15 +389,7 @@ fn build_columnar(docs: &[Vec<(&'static str, ColumnValue)>]) -> ColumnarReader {
     build_columnar_with_mapping(docs, None)
 }
 
-fn assert_columnar_eq_strict(left: &ColumnarReader, right: &ColumnarReader) {
-    assert_columnar_eq(left, right, false);
-}
-
-fn assert_columnar_eq(
-    left: &ColumnarReader,
-    right: &ColumnarReader,
-    lenient_on_numerical_value: bool,
-) {
+fn assert_columnar_eq(left: &ColumnarReader, right: &ColumnarReader) {
     assert_eq!(left.num_rows(), right.num_rows());
     let left_columns = left.list_columns().unwrap();
     let right_columns = right.list_columns().unwrap();
@@ -432,7 +398,7 @@ fn assert_columnar_eq(
         assert_eq!(left_columns[i].0, right_columns[i].0);
         let left_column = left_columns[i].1.open().unwrap();
         let right_column = right_columns[i].1.open().unwrap();
-        assert_dyn_column_eq(&left_column, &right_column, lenient_on_numerical_value);
+        assert_dyn_column_eq(&left_column, &right_column);
     }
 }
 
@@ -476,11 +442,11 @@ fn assert_bytes_column_eq(left: &BytesColumn, right: &BytesColumn) {
     assert!(!right_terms.advance());
 }
 
-fn assert_dyn_column_eq(
-    left_dyn_column: &DynamicColumn,
-    right_dyn_column: &DynamicColumn,
-    lenient_on_numerical_value: bool,
-) {
+fn assert_dyn_column_eq(left_dyn_column: &DynamicColumn, right_dyn_column: &DynamicColumn) {
+    assert_eq!(
+        &left_dyn_column.column_type(),
+        &right_dyn_column.column_type()
+    );
     assert_eq!(
         &left_dyn_column.get_cardinality(),
         &right_dyn_column.get_cardinality()
@@ -510,19 +476,8 @@ fn assert_dyn_column_eq(
         (DynamicColumn::Str(left_col), DynamicColumn::Str(right_col)) => {
             assert_bytes_column_eq(left_col, right_col);
         }
-        (left, right) => {
-            if lenient_on_numerical_value {
-                assert_eq!(
-                    ColumnTypeCategory::from(left.column_type()),
-                    ColumnTypeCategory::from(right.column_type())
-                );
-            } else {
-                panic!(
-                    "Column type are not the same: {:?} vs {:?}",
-                    left.column_type(),
-                    right.column_type()
-                );
-            }
+        _ => {
+            unreachable!()
         }
     }
 }
@@ -728,7 +683,7 @@ proptest! {
         let merged_columnar = ColumnarReader::open(output).unwrap();
         let concat_rows: Vec<Vec<(&'static str, ColumnValue)>> = columnar_docs.iter().cloned().flatten().collect();
         let expected_merged_columnar = build_columnar(&concat_rows[..]);
-        assert_columnar_eq_strict(&merged_columnar, &expected_merged_columnar);
+        assert_columnar_eq(&merged_columnar, &expected_merged_columnar);
     }
 }
 
@@ -754,7 +709,7 @@ fn test_columnar_merging_empty_columnar() {
     let concat_rows: Vec<Vec<(&'static str, ColumnValue)>> =
         columnar_docs.iter().cloned().flatten().collect();
     let expected_merged_columnar = build_columnar(&concat_rows[..]);
-    assert_columnar_eq_strict(&merged_columnar, &expected_merged_columnar);
+    assert_columnar_eq(&merged_columnar, &expected_merged_columnar);
 }
 
 #[test]
@@ -791,135 +746,8 @@ fn test_columnar_merging_number_columns() {
     let concat_rows: Vec<Vec<(&'static str, ColumnValue)>> =
         columnar_docs.iter().cloned().flatten().collect();
     let expected_merged_columnar = build_columnar(&concat_rows[..]);
-    assert_columnar_eq_strict(&merged_columnar, &expected_merged_columnar);
+    assert_columnar_eq(&merged_columnar, &expected_merged_columnar);
 }
-
 // TODO add non trivial remap and merge
 // TODO test required_columns
 // TODO document edge case: required_columns incompatible with values.
-
-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(
-        |columnars_docs: Vec<Vec<Vec<(&str, ColumnValue)>>>| {
-            let row_addrs: Vec<RowAddr> = columnars_docs
-                .iter()
-                .enumerate()
-                .flat_map(|(segment_ord, columnar_docs)| {
-                    (0u32..columnar_docs.len() as u32).map(move |row_id| RowAddr {
-                        segment_ord: segment_ord as u32,
-                        row_id,
-                    })
-                })
-                .collect();
-            permutation_and_subset_strategy(row_addrs.len()).prop_map(move |shuffled_subset| {
-                let shuffled_row_addr_subset: Vec<RowAddr> =
-                    shuffled_subset.iter().map(|ord| row_addrs[*ord]).collect();
-                (columnars_docs.clone(), shuffled_row_addr_subset)
-            })
-        },
-    )
-}
-
-proptest! {
-    #![proptest_config(ProptestConfig::with_cases(1000))]
-    #[test]
-    fn test_columnar_merge_and_remap_proptest((columnar_docs, shuffle_merge_order) in columnar_docs_and_remap()) {
-        let shuffled_rows: Vec<Vec<(&'static str, ColumnValue)>> = shuffle_merge_order.iter()
-            .map(|row_addr| columnar_docs[row_addr.segment_ord as usize][row_addr.row_id as usize].clone())
-            .collect();
-        let expected_merged_columnar = build_columnar(&shuffled_rows[..]);
-        let columnar_readers: Vec<ColumnarReader> = columnar_docs.iter()
-            .map(|docs| build_columnar(&docs[..]))
-            .collect::<Vec<_>>();
-        let columnar_readers_arr: Vec<&ColumnarReader> = columnar_readers.iter().collect();
-        let mut output: Vec<u8> = Vec::new();
-        let segment_num_rows: Vec<RowId> = columnar_docs.iter().map(|docs| docs.len() as RowId).collect();
-        let shuffle_merge_order = ShuffleMergeOrder::for_test(&segment_num_rows, shuffle_merge_order);
-        crate::merge_columnar(&columnar_readers_arr[..], &[], shuffle_merge_order.into(), &mut output).unwrap();
-        let merged_columnar = ColumnarReader::open(output).unwrap();
-        assert_columnar_eq(&merged_columnar, &expected_merged_columnar, true);
-    }
-}
-
-#[test]
-fn test_columnar_merge_empty() {
-    let columnar_reader_1 = build_columnar(&[]);
-    let rows: &[Vec<_>] = &[vec![("c1", ColumnValue::Str("a"))]][..];
-    let columnar_reader_2 = build_columnar(rows);
-    let mut output: Vec<u8> = Vec::new();
-    let segment_num_rows: Vec<RowId> = vec![0, 0];
-    let shuffle_merge_order = ShuffleMergeOrder::for_test(&segment_num_rows, vec![]);
-    crate::merge_columnar(
-        &[&columnar_reader_1, &columnar_reader_2],
-        &[],
-        shuffle_merge_order.into(),
-        &mut output,
-    )
-    .unwrap();
-    let merged_columnar = ColumnarReader::open(output).unwrap();
-    assert_eq!(merged_columnar.num_rows(), 0);
-    assert_eq!(merged_columnar.num_columns(), 0);
-}
-
-#[test]
-fn test_columnar_merge_single_str_column() {
-    let columnar_reader_1 = build_columnar(&[]);
-    let rows: &[Vec<_>] = &[vec![("c1", ColumnValue::Str("a"))]][..];
-    let columnar_reader_2 = build_columnar(rows);
-    let mut output: Vec<u8> = Vec::new();
-    let segment_num_rows: Vec<RowId> = vec![0, 1];
-    let shuffle_merge_order = ShuffleMergeOrder::for_test(
-        &segment_num_rows,
-        vec![RowAddr {
-            segment_ord: 1u32,
-            row_id: 0u32,
-        }],
-    );
-    crate::merge_columnar(
-        &[&columnar_reader_1, &columnar_reader_2],
-        &[],
-        shuffle_merge_order.into(),
-        &mut output,
-    )
-    .unwrap();
-    let merged_columnar = ColumnarReader::open(output).unwrap();
-    assert_eq!(merged_columnar.num_rows(), 1);
-    assert_eq!(merged_columnar.num_columns(), 1);
-}
-
-#[test]
-fn test_delete_decrease_cardinality() {
-    let columnar_reader_1 = build_columnar(&[]);
-    let rows: &[Vec<_>] = &[
-        vec![
-            ("c", ColumnValue::from(0i64)),
-            ("c", ColumnValue::from(0i64)),
-        ],
-        vec![("c", ColumnValue::from(0i64))],
-    ][..];
-    // c is multivalued here
-    let columnar_reader_2 = build_columnar(rows);
-    let mut output: Vec<u8> = Vec::new();
-    let shuffle_merge_order = ShuffleMergeOrder::for_test(
-        &[0, 2],
-        vec![RowAddr {
-            segment_ord: 1u32,
-            row_id: 1u32,
-        }],
-    );
-    crate::merge_columnar(
-        &[&columnar_reader_1, &columnar_reader_2],
-        &[],
-        shuffle_merge_order.into(),
-        &mut output,
-    )
-    .unwrap();
-    let merged_columnar = ColumnarReader::open(output).unwrap();
-    assert_eq!(merged_columnar.num_rows(), 1);
-    assert_eq!(merged_columnar.num_columns(), 1);
-    let cols = merged_columnar.read_columns("c").unwrap();
-    assert_eq!(cols.len(), 1);
-    assert_eq!(cols[0].column_type(), ColumnType::I64);
-    assert_eq!(cols[0].open().unwrap().get_cardinality(), Cardinality::Full);
-}

src/fastfield/mod.rs

@@ -14,8 +14,8 @@
 //! Fields have to be declared as `FAST` in the schema.
 //! Currently supported fields are: u64, i64, f64, bytes, ip and text.
 //!
-//! Fast fields are stored in with [different codecs](fastfield_codecs). The best codec is detected
-//! automatically, when serializing.
+//! Fast fields are stored in with [different codecs](columnar::column_values). The best codec is
+//! detected automatically, when serializing.
 //!
 //! Read access performance is comparable to that of an array lookup.
 

src/query/boolean_query/block_wand.rs

@@ -228,7 +228,6 @@ pub fn block_wand_single_scorer(
     loop {
         // We position the scorer on a block that can reach
         // the threshold.
-        dbg!(scorer.block_max_score(), threshold);
         while scorer.block_max_score() < threshold {
             let last_doc_in_block = scorer.last_doc_in_block();
             if last_doc_in_block == TERMINATED {
@@ -359,7 +358,6 @@ mod tests {
             }
             limit
         };
-        dbg!(term_scorers.len());
 
         if term_scorers.len() == 1 {
             let scorer = term_scorers.pop().unwrap();
@@ -433,7 +431,7 @@ mod tests {
 
     fn test_block_wand_aux(posting_lists: &[Vec<(DocId, u32)>], fieldnorms: &[u32]) {
         // We virtually repeat all docs 64 times in order to emulate blocks of 2 documents
-        // and surface bugs more easily.
+        // and surface blogs more easily.
         const REPEAT: usize = 64;
         let fieldnorms_expanded = fieldnorms
             .iter()
@@ -478,13 +476,10 @@ mod tests {
                 TermScorer::create_for_test(postings, &fieldnorms_expanded[..], bm25_weight)
             })
             .collect();
-        for top_k in 2..3 {
+        for top_k in 1..4 {
             let checkpoints_for_each_pruning =
                 compute_checkpoints_for_each_pruning(term_scorers.clone(), top_k);
-            let checkpoints_manual =
-                compute_checkpoints_manual(term_scorers.clone(), top_k);
-            dbg!(&checkpoints_for_each_pruning);
-            dbg!(&checkpoints_manual);
+            let checkpoints_manual = compute_checkpoints_manual(term_scorers.clone(), top_k);
             assert_eq!(checkpoints_for_each_pruning.len(), checkpoints_manual.len());
             for (&(left_doc, left_score), &(right_doc, right_score)) in checkpoints_for_each_pruning
                 .iter()
@@ -512,13 +507,6 @@ mod tests {
         }
     }
 
-    #[test]
-    fn test_block_wand_failing_case() {
-        let posting_lists = &[vec![(0, 94), (1, 47), (2, 20)]];
-        let field_norms = &[80, 33, 2];
-        test_block_wand_aux(&posting_lists[..], field_norms);
-    }
-
     #[test]
     fn test_fn_reproduce_proptest() {
         let postings_lists = &[

src/tokenizer/simple_tokenizer.rs

@@ -2,7 +2,8 @@ use std::str::CharIndices;
 
 use super::{Token, TokenStream, Tokenizer};
 
-/// Tokenize the text by splitting on whitespaces and punctuation.
+/// Tokenize the text by returning only tokens of consecutive
+/// [`alphanumeric`](char::is_alphanumeric).
 #[derive(Clone)]
 pub struct SimpleTokenizer;
 

src/tokenizer/tokenizer_manager.rs

@@ -13,9 +13,8 @@ use crate::tokenizer::{
 /// By default, it is populated with the following managers.
 ///
 ///  * `raw` : does not process nor tokenize the text.
-///  * `default` : Chops the text on according to whitespace and
-///  punctuation, removes tokens that are too long, and lowercases
-///  tokens
+///  * `default` : Chops the text according to [`SimpleTokenizer`],
+///  removes tokens that are longer than 40, and lowercases tokens
 ///  * `en_stem` : Like `default`, but also applies stemming on the
 ///  resulting tokens. Stemming can improve the recall of your
 ///  search engine.
@@ -35,7 +34,9 @@ impl TokenizerManager {
 
     /// Registers a new tokenizer associated with a given name.
     pub fn register<T>(&self, tokenizer_name: &str, tokenizer: T)
-    where TextAnalyzer: From<T> {
+    where
+        TextAnalyzer: From<T>,
+    {
         let boxed_tokenizer: TextAnalyzer = TextAnalyzer::from(tokenizer);
         self.tokenizers
             .write()

sstable/src/dictionary.rs

@@ -427,7 +427,7 @@ mod tests {
         // this makes 256k keys, enough to fill multiple blocks.
         for elem in 0..0x3ffff {
             let key = format!("{elem:05X}").into_bytes();
-            builder.insert(&key, &elem).unwrap();
+            builder.insert_cannot_fail(&key, &elem);
         }
 
         let table = builder.finish().unwrap();

sstable/src/lib.rs

@@ -318,6 +318,16 @@ where
     }
 }
 
+impl<TValueWriter> Writer<Vec<u8>, TValueWriter>
+where TValueWriter: value::ValueWriter
+{
+    #[inline]
+    pub fn insert_cannot_fail<K: AsRef<[u8]>>(&mut self, key: K, value: &TValueWriter::Value) {
+        self.insert(key, value)
+            .expect("SSTable over a Vec should never fail");
+    }
+}
+
 #[cfg(test)]
 mod test {
     use std::io;

sstable/src/streamer.rs

@@ -158,22 +158,6 @@ where
     upper_bound: Bound<Vec<u8>>,
 }
 
-impl<'a, TSSTable> Streamer<'a, TSSTable, AlwaysMatch>
-where TSSTable: SSTable
-{
-    pub fn empty() -> Self {
-        Streamer {
-            automaton: AlwaysMatch,
-            states: Vec::new(),
-            delta_reader: DeltaReader::empty(),
-            key: Vec::new(),
-            term_ord: None,
-            lower_bound: Bound::Unbounded,
-            upper_bound: Bound::Unbounded,
-        }
-    }
-}
-
 impl<'a, TSSTable, A> Streamer<'a, TSSTable, A>
 where
     A: Automaton,