Merge #594

594: Fix(Search): Fix phrase search candidates computation r=Kerollmops a=ManyTheFish

This bug is an old bug but was hidden by the proximity criterion,
Phrase searches were always returning an empty candidates list when the proximity criterion is deactivated.

Before the fix, we were trying to find any words[n] near words[n]
instead of finding  any words[n] near words[n+1], for example:

for a phrase search '"Hello world"' we were searching for "hello" near "hello" first, instead of "hello" near "world".



Co-authored-by: ManyTheFish <many@meilisearch.com>

milli/src/search/criteria/mod.rs

@@ -293,13 +293,13 @@ impl<'t> CriteriaBuilder<'t> {
     }
 }
 
-pub fn resolve_query_tree<'t>(
+pub fn resolve_query_tree(
-    ctx: &'t dyn Context,
+    ctx: &dyn Context,
     query_tree: &Operation,
     wdcache: &mut WordDerivationsCache,
 ) -> Result<RoaringBitmap> {
-    fn resolve_operation<'t>(
+    fn resolve_operation(
-        ctx: &'t dyn Context,
+        ctx: &dyn Context,
         query_tree: &Operation,
         wdcache: &mut WordDerivationsCache,
     ) -> Result<RoaringBitmap> {
@@ -326,43 +326,7 @@ pub fn resolve_query_tree<'t>(
                 }
                 Ok(candidates)
             }
-            Phrase(words) => {
+            Phrase(words) => resolve_phrase(ctx, &words),
-                let mut candidates = RoaringBitmap::new();
-                let mut first_iter = true;
-                let winsize = words.len().min(7);
-
-                for win in words.windows(winsize) {
-                    // Get all the documents with the matching distance for each word pairs.
-                    let mut bitmaps = Vec::with_capacity(winsize.pow(2));
-                    for (offset, s1) in win.iter().enumerate() {
-                        for (dist, s2) in win.iter().skip(offset).enumerate() {
-                            match ctx.word_pair_proximity_docids(s1, s2, dist as u8 + 1)? {
-                                Some(m) => bitmaps.push(m),
-                                // If there are no document for this distance, there will be no
-                                // results for the phrase query.
-                                None => return Ok(RoaringBitmap::new()),
-                            }
-                        }
-                    }
-
-                    // We sort the bitmaps so that we perform the small intersections first, which is faster.
-                    bitmaps.sort_unstable_by(|a, b| a.len().cmp(&b.len()));
-
-                    for bitmap in bitmaps {
-                        if first_iter {
-                            candidates = bitmap;
-                            first_iter = false;
-                        } else {
-                            candidates &= bitmap;
-                        }
-                        // There will be no match, return early
-                        if candidates.is_empty() {
-                            break;
-                        }
-                    }
-                }
-                Ok(candidates)
-            }
             Or(_, ops) => {
                 let mut candidates = RoaringBitmap::new();
                 for op in ops {
@@ -378,6 +342,44 @@ pub fn resolve_query_tree<'t>(
     resolve_operation(ctx, query_tree, wdcache)
 }
 
+pub fn resolve_phrase(ctx: &dyn Context, phrase: &[String]) -> Result<RoaringBitmap> {
+    let mut candidates = RoaringBitmap::new();
+    let mut first_iter = true;
+    let winsize = phrase.len().min(7);
+
+    for win in phrase.windows(winsize) {
+        // Get all the documents with the matching distance for each word pairs.
+        let mut bitmaps = Vec::with_capacity(winsize.pow(2));
+        for (offset, s1) in win.iter().enumerate() {
+            for (dist, s2) in win.iter().skip(offset + 1).enumerate() {
+                match ctx.word_pair_proximity_docids(s1, s2, dist as u8 + 1)? {
+                    Some(m) => bitmaps.push(m),
+                    // If there are no document for this distance, there will be no
+                    // results for the phrase query.
+                    None => return Ok(RoaringBitmap::new()),
+                }
+            }
+        }
+
+        // We sort the bitmaps so that we perform the small intersections first, which is faster.
+        bitmaps.sort_unstable_by(|a, b| a.len().cmp(&b.len()));
+
+        for bitmap in bitmaps {
+            if first_iter {
+                candidates = bitmap;
+                first_iter = false;
+            } else {
+                candidates &= bitmap;
+            }
+            // There will be no match, return early
+            if candidates.is_empty() {
+                break;
+            }
+        }
+    }
+    Ok(candidates)
+}
+
 fn all_word_pair_proximity_docids<T: AsRef<str>, U: AsRef<str>>(
     ctx: &dyn Context,
     left_words: &[(T, u8)],

milli/src/search/criteria/proximity.rs

@@ -6,8 +6,8 @@ use log::debug;
 use roaring::RoaringBitmap;
 
 use super::{
-    query_docids, query_pair_proximity_docids, resolve_query_tree, Context, Criterion,
+    query_docids, query_pair_proximity_docids, resolve_phrase, resolve_query_tree, Context,
-    CriterionParameters, CriterionResult,
+    Criterion, CriterionParameters, CriterionResult,
 };
 use crate::search::query_tree::{maximum_proximity, Operation, Query, QueryKind};
 use crate::search::{build_dfa, WordDerivationsCache};
@@ -192,22 +192,9 @@ fn resolve_candidates<'t>(
                     let most_right = words
                         .last()
                         .map(|w| Query { prefix: false, kind: QueryKind::exact(w.clone()) });
-                    let mut candidates = None;
+
-                    for slice in words.windows(2) {
+                    match (most_left, most_right) {
-                        let (left, right) = (&slice[0], &slice[1]);
+                        (Some(l), Some(r)) => vec![(l, r, resolve_phrase(ctx, &words)?)],
-                        match ctx.word_pair_proximity_docids(left, right, 1)? {
-                            Some(pair_docids) => match candidates.as_mut() {
-                                Some(candidates) => *candidates &= pair_docids,
-                                None => candidates = Some(pair_docids),
-                            },
-                            None => {
-                                candidates = None;
-                                break;
-                            }
-                        }
-                    }
-                    match (most_left, most_right, candidates) {
-                        (Some(l), Some(r), Some(c)) => vec![(l, r, c)],
                         _otherwise => Default::default(),
                     }
                 } else {