From 3421125a553c933fae6c439f60daf2b430389cf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Lecrenier?= Date: Tue, 25 Apr 2023 17:52:42 +0200 Subject: [PATCH] Prevent the `exactness` ranking rule from removing random words Make it strictly follow the term matching strategy --- milli/src/search/new/mod.rs | 101 +++--------------- .../new/ranking_rule_graph/exactness/mod.rs | 11 +- milli/src/search/new/tests/exactness.rs | 94 +++++++--------- 3 files changed, 61 insertions(+), 145 deletions(-) diff --git a/milli/src/search/new/mod.rs b/milli/src/search/new/mod.rs index fb96af5e7..e9518bad5 100644 --- a/milli/src/search/new/mod.rs +++ b/milli/src/search/new/mod.rs @@ -51,7 +51,6 @@ use resolve_query_graph::compute_query_graph_docids; use sort::Sort; use self::interner::Interned; -use self::query_term::ExactTerm; /// A structure used throughout the execution of a search query. pub struct SearchContext<'ctx> { @@ -120,73 +119,20 @@ fn resolve_maximally_reduced_query_graph( Ok(docids) } -fn resolve_docids_containing_any_exact_word( - ctx: &mut SearchContext, - universe: &RoaringBitmap, - query_graph: &QueryGraph, -) -> Result { - let mut docids = RoaringBitmap::new(); - for (_, node) in query_graph.nodes.iter() { - let term = match &node.data { - query_graph::QueryNodeData::Term(term) => term, - query_graph::QueryNodeData::Deleted - | query_graph::QueryNodeData::Start - | query_graph::QueryNodeData::End => { - continue; - } - }; - if term.term_ids.len() != 1 { - continue; - } - let Some(exact_term) = term.term_subset.exact_term(ctx) else { - continue - }; - let exact_term_docids = match exact_term { - ExactTerm::Phrase(phrase) => ctx.get_phrase_docids(phrase)? & universe, - ExactTerm::Word(word) => { - if let Some(word_docids) = ctx.word_docids(Word::Original(word))? { - word_docids & universe - } else { - continue; - } - } - }; - docids |= exact_term_docids; - } - Ok(docids) -} - fn resolve_universe( ctx: &mut SearchContext, initial_universe: &RoaringBitmap, query_graph: &QueryGraph, - method: UniverseResolutionMethod, matching_strategy: TermsMatchingStrategy, logger: &mut dyn SearchLogger, ) -> Result { - match method { - UniverseResolutionMethod::TermMatchingStrategyOnly => { - resolve_maximally_reduced_query_graph( - ctx, - initial_universe, - query_graph, - matching_strategy, - logger, - ) - } - UniverseResolutionMethod::TermMatchingStrategyAndExactness => { - let mut resolved_universe = resolve_maximally_reduced_query_graph( - ctx, - initial_universe, - query_graph, - matching_strategy, - logger, - )?; - resolved_universe |= - resolve_docids_containing_any_exact_word(ctx, initial_universe, query_graph)?; - Ok(resolved_universe) - } - } + resolve_maximally_reduced_query_graph( + ctx, + initial_universe, + query_graph, + matching_strategy, + logger, + ) } /// Return the list of initialised ranking rules to be used for a placeholder search. @@ -233,17 +179,12 @@ fn get_ranking_rules_for_placeholder_search<'ctx>( Ok(ranking_rules) } -enum UniverseResolutionMethod { - TermMatchingStrategyOnly, - TermMatchingStrategyAndExactness, -} - /// Return the list of initialised ranking rules to be used for a query graph search. fn get_ranking_rules_for_query_graph_search<'ctx>( ctx: &SearchContext<'ctx>, sort_criteria: &Option>, terms_matching_strategy: TermsMatchingStrategy, -) -> Result<(Vec>, UniverseResolutionMethod)> { +) -> Result>> { // query graph search let mut words = false; let mut typo = false; @@ -254,14 +195,15 @@ fn get_ranking_rules_for_query_graph_search<'ctx>( let mut asc = HashSet::new(); let mut desc = HashSet::new(); - let mut universe_resolution_method = UniverseResolutionMethod::TermMatchingStrategyOnly; - let mut ranking_rules: Vec> = vec![]; let settings_ranking_rules = ctx.index.criteria(ctx.txn)?; for rr in settings_ranking_rules { // Add Words before any of: typo, proximity, attribute match rr { - crate::Criterion::Typo | crate::Criterion::Attribute | crate::Criterion::Proximity => { + crate::Criterion::Typo + | crate::Criterion::Attribute + | crate::Criterion::Proximity + | crate::Criterion::Exactness => { if !words { ranking_rules.push(Box::new(Words::new(terms_matching_strategy))); words = true; @@ -313,11 +255,6 @@ fn get_ranking_rules_for_query_graph_search<'ctx>( ranking_rules.push(Box::new(ExactAttribute::new())); ranking_rules.push(Box::new(Exactness::new())); exactness = true; - - if !words { - universe_resolution_method = - UniverseResolutionMethod::TermMatchingStrategyAndExactness; - } } crate::Criterion::Asc(field_name) => { if asc.contains(&field_name) { @@ -335,7 +272,7 @@ fn get_ranking_rules_for_query_graph_search<'ctx>( } } } - Ok((ranking_rules, universe_resolution_method)) + Ok(ranking_rules) } fn resolve_sort_criteria<'ctx, Query: RankingRuleQueryTrait>( @@ -417,17 +354,11 @@ pub fn execute_search( check_sort_criteria(ctx, sort_criteria.as_ref())?; - let (ranking_rules, universe_resolution_method) = + let ranking_rules = get_ranking_rules_for_query_graph_search(ctx, sort_criteria, terms_matching_strategy)?; - universe = resolve_universe( - ctx, - &universe, - &graph, - universe_resolution_method, - terms_matching_strategy, - query_graph_logger, - )?; + universe = + resolve_universe(ctx, &universe, &graph, terms_matching_strategy, query_graph_logger)?; bucket_sort(ctx, ranking_rules, &graph, &universe, from, length, query_graph_logger)? } else { diff --git a/milli/src/search/new/ranking_rule_graph/exactness/mod.rs b/milli/src/search/new/ranking_rule_graph/exactness/mod.rs index 431eeac30..0842d6d04 100644 --- a/milli/src/search/new/ranking_rule_graph/exactness/mod.rs +++ b/milli/src/search/new/ranking_rule_graph/exactness/mod.rs @@ -3,13 +3,14 @@ use roaring::RoaringBitmap; use super::{ComputedCondition, RankingRuleGraphTrait}; use crate::search::new::interner::{DedupInterner, Interned}; use crate::search::new::query_term::{ExactTerm, LocatedQueryTermSubset}; +use crate::search::new::resolve_query_graph::compute_query_term_subset_docids; use crate::search::new::Word; use crate::{Result, SearchContext}; #[derive(Clone, PartialEq, Eq, Hash)] pub enum ExactnessCondition { ExactInAttribute(LocatedQueryTermSubset), - Skip(LocatedQueryTermSubset), + Any(LocatedQueryTermSubset), } pub enum ExactnessGraph {} @@ -54,7 +55,11 @@ impl RankingRuleGraphTrait for ExactnessGraph { end_term_subset.term_subset.make_mandatory(); (compute_docids(ctx, dest_node, universe)?, end_term_subset) } - ExactnessCondition::Skip(dest_node) => (universe.clone(), dest_node.clone()), + ExactnessCondition::Any(dest_node) => { + let docids = + universe & compute_query_term_subset_docids(ctx, &dest_node.term_subset)?; + (docids, dest_node.clone()) + } }; Ok(ComputedCondition { @@ -74,7 +79,7 @@ impl RankingRuleGraphTrait for ExactnessGraph { let exact_condition = ExactnessCondition::ExactInAttribute(dest_node.clone()); let exact_condition = conditions_interner.insert(exact_condition); - let skip_condition = ExactnessCondition::Skip(dest_node.clone()); + let skip_condition = ExactnessCondition::Any(dest_node.clone()); let skip_condition = conditions_interner.insert(skip_condition); Ok(vec![(0, exact_condition), (dest_node.term_ids.len() as u32, skip_condition)]) diff --git a/milli/src/search/new/tests/exactness.rs b/milli/src/search/new/tests/exactness.rs index 2077f6c01..fd6bcf0af 100644 --- a/milli/src/search/new/tests/exactness.rs +++ b/milli/src/search/new/tests/exactness.rs @@ -6,19 +6,17 @@ This module tests the following properties about the exactness ranking rule: 2. documents which have an attribute which start with the whole query 3. documents which contain the most exact words from the query -- the set of all candidates when `exactness` precedes `word` is the union of: - 1. the same set of candidates that would be returned normally - 2. the set of documents that contain at least one exact word from the query +- the `exactness` ranking rule must be preceded by the `words` ranking rule -- if it is placed after `word`, then it will only sort documents by: +- if `words` has already removed terms from the query, then exactness will sort documents as follows: 1. those that have an attribute which is equal to the whole remaining query, if this query does not have any "gap" 2. those that have an attribute which start with the whole remaining query, if this query does not have any "gap" 3. those that contain the most exact words from the remaining query -- if it is followed by other ranking rules, then: - 1. `word` will not remove the exact terms matched by `exactness` - 2. graph-based ranking rules (`typo`, `proximity`, `attribute`) will only work with - (1) the exact terms selected by `exactness` or (2) the full query term otherwise +- if it is followed by other graph-based ranking rules (`typo`, `proximity`, `attribute`). +Then these rules will only work with + 1. the exact terms selected by `exactness + 2. the full query term otherwise */ use crate::{ @@ -440,14 +438,14 @@ fn test_exactness_simple_ordered() { s.terms_matching_strategy(TermsMatchingStrategy::Last); s.query("the quick brown fox jumps over the lazy dog"); let SearchResult { documents_ids, .. } = s.execute().unwrap(); - insta::assert_snapshot!(format!("{documents_ids:?}"), @"[9, 8, 6, 7, 5, 4, 3, 2, 1]"); + insta::assert_snapshot!(format!("{documents_ids:?}"), @"[9, 8, 7, 6, 5, 4, 3, 2, 1]"); let texts = collect_field_values(&index, &txn, "text", &documents_ids); insta::assert_debug_snapshot!(texts, @r###" [ "\"the quick brown fox jumps over the lazy dog\"", "\"the quick brown fox jumps over the lazy\"", - "\"the quick brown fox jumps over\"", "\"the quick brown fox jumps over the\"", + "\"the quick brown fox jumps over\"", "\"the quick brown fox jumps\"", "\"the quick brown fox\"", "\"the quick brown\"", @@ -467,19 +465,17 @@ fn test_exactness_simple_reversed() { s.terms_matching_strategy(TermsMatchingStrategy::Last); s.query("the quick brown fox jumps over the lazy dog"); let SearchResult { documents_ids, .. } = s.execute().unwrap(); - insta::assert_snapshot!(format!("{documents_ids:?}"), @"[9, 8, 7, 6, 5, 4, 3, 2, 1]"); + insta::assert_snapshot!(format!("{documents_ids:?}"), @"[9, 8, 3, 4, 5, 6, 7]"); let texts = collect_field_values(&index, &txn, "text", &documents_ids); insta::assert_debug_snapshot!(texts, @r###" [ "\"the quick brown fox jumps over the lazy dog\"", "\"quick brown fox jumps over the lazy dog\"", - "\"brown fox jumps over the lazy dog\"", - "\"fox jumps over the lazy dog\"", - "\"jumps over the lazy dog\"", - "\"over the lazy dog\"", "\"the lazy dog\"", - "\"lazy dog\"", - "\"dog\"", + "\"over the lazy dog\"", + "\"jumps over the lazy dog\"", + "\"fox jumps over the lazy dog\"", + "\"brown fox jumps over the lazy dog\"", ] "###); @@ -487,19 +483,17 @@ fn test_exactness_simple_reversed() { s.terms_matching_strategy(TermsMatchingStrategy::Last); s.query("the quick brown fox jumps over the lazy dog"); let SearchResult { documents_ids, .. } = s.execute().unwrap(); - insta::assert_snapshot!(format!("{documents_ids:?}"), @"[9, 8, 7, 6, 5, 4, 3, 2, 1]"); + insta::assert_snapshot!(format!("{documents_ids:?}"), @"[9, 8, 3, 4, 5, 6, 7]"); let texts = collect_field_values(&index, &txn, "text", &documents_ids); insta::assert_debug_snapshot!(texts, @r###" [ "\"the quick brown fox jumps over the lazy dog\"", "\"quick brown fox jumps over the lazy dog\"", - "\"brown fox jumps over the lazy dog\"", - "\"fox jumps over the lazy dog\"", - "\"jumps over the lazy dog\"", - "\"over the lazy dog\"", "\"the lazy dog\"", - "\"lazy dog\"", - "\"dog\"", + "\"over the lazy dog\"", + "\"jumps over the lazy dog\"", + "\"fox jumps over the lazy dog\"", + "\"brown fox jumps over the lazy dog\"", ] "###); } @@ -514,18 +508,16 @@ fn test_exactness_simple_random() { s.terms_matching_strategy(TermsMatchingStrategy::Last); s.query("the quick brown fox jumps over the lazy dog"); let SearchResult { documents_ids, .. } = s.execute().unwrap(); - insta::assert_snapshot!(format!("{documents_ids:?}"), @"[8, 7, 5, 6, 3, 4, 1, 2]"); + insta::assert_snapshot!(format!("{documents_ids:?}"), @"[8, 7, 4, 6, 3, 5]"); let texts = collect_field_values(&index, &txn, "text", &documents_ids); insta::assert_debug_snapshot!(texts, @r###" [ "\"the jumps dog quick over brown lazy fox\"", "\"the dog brown over jumps quick lazy\"", - "\"fox the lazy dog brown\"", + "\"jump dog quick the\"", "\"jump fox quick lazy the dog\"", "\"brown the lazy\"", - "\"jump dog quick the\"", - "\"over\"", - "\"jump dog\"", + "\"fox the lazy dog brown\"", ] "###); } @@ -540,17 +532,13 @@ fn test_exactness_attribute_starts_with_simple() { s.terms_matching_strategy(TermsMatchingStrategy::Last); s.query("this balcony"); let SearchResult { documents_ids, .. } = s.execute().unwrap(); - insta::assert_snapshot!(format!("{documents_ids:?}"), @"[2, 1, 0, 3, 4, 5, 6]"); + insta::assert_snapshot!(format!("{documents_ids:?}"), @"[2, 1, 0]"); let texts = collect_field_values(&index, &txn, "text", &documents_ids); insta::assert_debug_snapshot!(texts, @r###" [ "\"this balcony\"", "\"this balcony is overlooking the sea\"", "\"what a lovely view from this balcony, I love it\"", - "\"over looking the sea is a beautiful balcony\"", - "\"a beautiful balcony is overlooking the sea\"", - "\"overlooking the sea is a beautiful balcony, I love it\"", - "\"overlooking the sea is a beautiful balcony\"", ] "###); } @@ -565,17 +553,14 @@ fn test_exactness_attribute_starts_with_phrase() { s.terms_matching_strategy(TermsMatchingStrategy::Last); s.query("\"overlooking the sea\" is a beautiful balcony"); let SearchResult { documents_ids, .. } = s.execute().unwrap(); - insta::assert_snapshot!(format!("{documents_ids:?}"), @"[6, 5, 4, 3, 1, 0, 2]"); + insta::assert_snapshot!(format!("{documents_ids:?}"), @"[6, 5, 4, 1]"); let texts = collect_field_values(&index, &txn, "text", &documents_ids); insta::assert_debug_snapshot!(texts, @r###" [ "\"overlooking the sea is a beautiful balcony\"", "\"overlooking the sea is a beautiful balcony, I love it\"", "\"a beautiful balcony is overlooking the sea\"", - "\"over looking the sea is a beautiful balcony\"", "\"this balcony is overlooking the sea\"", - "\"what a lovely view from this balcony, I love it\"", - "\"this balcony\"", ] "###); @@ -583,7 +568,7 @@ fn test_exactness_attribute_starts_with_phrase() { s.terms_matching_strategy(TermsMatchingStrategy::Last); s.query("overlooking the sea is a beautiful balcony"); let SearchResult { documents_ids, .. } = s.execute().unwrap(); - insta::assert_snapshot!(format!("{documents_ids:?}"), @"[6, 5, 4, 3, 1, 0, 2, 7]"); + insta::assert_snapshot!(format!("{documents_ids:?}"), @"[6, 5, 4, 3, 1, 7]"); let texts = collect_field_values(&index, &txn, "text", &documents_ids); insta::assert_debug_snapshot!(texts, @r###" [ @@ -592,8 +577,6 @@ fn test_exactness_attribute_starts_with_phrase() { "\"a beautiful balcony is overlooking the sea\"", "\"over looking the sea is a beautiful balcony\"", "\"this balcony is overlooking the sea\"", - "\"what a lovely view from this balcony, I love it\"", - "\"this balcony\"", "\"overlooking\"", ] "###); @@ -609,19 +592,16 @@ fn test_exactness_all_candidates_with_typo() { s.terms_matching_strategy(TermsMatchingStrategy::Last); s.query("overlocking the sea is a beautiful balcony"); let SearchResult { documents_ids, .. } = s.execute().unwrap(); - insta::assert_snapshot!(format!("{documents_ids:?}"), @"[3, 4, 5, 6, 1, 0, 2, 7]"); + insta::assert_snapshot!(format!("{documents_ids:?}"), @"[4, 5, 6, 1, 7]"); let texts = collect_field_values(&index, &txn, "text", &documents_ids); // "overlooking" is returned here because the term matching strategy allows it // but it has the worst exactness score (0 exact words) insta::assert_debug_snapshot!(texts, @r###" [ - "\"over looking the sea is a beautiful balcony\"", "\"a beautiful balcony is overlooking the sea\"", "\"overlooking the sea is a beautiful balcony, I love it\"", "\"overlooking the sea is a beautiful balcony\"", "\"this balcony is overlooking the sea\"", - "\"what a lovely view from this balcony, I love it\"", - "\"this balcony\"", "\"overlooking\"", ] "###); @@ -686,26 +666,26 @@ fn test_words_after_exactness() { s.terms_matching_strategy(TermsMatchingStrategy::Last); s.query("the quick brown fox jumps over the lazy dog"); let SearchResult { documents_ids, .. } = s.execute().unwrap(); - insta::assert_snapshot!(format!("{documents_ids:?}"), @"[19, 18, 16, 17, 9, 15, 8, 14, 6, 7, 13, 5, 4, 12, 3, 2, 1, 11]"); + insta::assert_snapshot!(format!("{documents_ids:?}"), @"[19, 9, 18, 8, 17, 16, 6, 7, 15, 5, 14, 4, 13, 3, 12, 2, 1, 11]"); let texts = collect_field_values(&index, &txn, "text", &documents_ids); insta::assert_debug_snapshot!(texts, @r###" [ "\"the quick brown fox jumps over the lazy dog\"", - "\"the quick brown fox jumps over the lazy\"", - "\"the quick brown fox jumps over\"", - "\"the quick brown fox jumps over the\"", "\"the quack briwn fox jlmps over the lazy dog\"", - "\"the quick brown fox jumps\"", + "\"the quick brown fox jumps over the lazy\"", "\"the quack briwn fox jlmps over the lazy\"", - "\"the quick brown fox\"", + "\"the quick brown fox jumps over the\"", + "\"the quick brown fox jumps over\"", "\"the quack briwn fox jlmps over\"", "\"the quack briwn fox jlmps over the\"", - "\"the quick brown\"", + "\"the quick brown fox jumps\"", "\"the quack briwn fox jlmps\"", + "\"the quick brown fox\"", "\"the quack briwn fox\"", - "\"the quick\"", + "\"the quick brown\"", "\"the quack briwn\"", + "\"the quick\"", "\"the quack\"", "\"the\"", "\"the\"", @@ -729,7 +709,7 @@ fn test_proximity_after_exactness() { s.terms_matching_strategy(TermsMatchingStrategy::Last); s.query("the quick brown fox jumps over the lazy dog"); let SearchResult { documents_ids, .. } = s.execute().unwrap(); - insta::assert_snapshot!(format!("{documents_ids:?}"), @"[2, 1, 0, 5, 4, 3, 8, 6, 7]"); + insta::assert_snapshot!(format!("{documents_ids:?}"), @"[2, 1, 0, 4, 5, 8, 7, 3, 6]"); let texts = collect_field_values(&index, &txn, "text", &documents_ids); insta::assert_debug_snapshot!(texts, @r###" @@ -737,12 +717,12 @@ fn test_proximity_after_exactness() { "\"the quick brown fox jumps over the lazy dog\"", "\"the quick brown fox jumps over the very lazy dog\"", "\"lazy jumps dog brown quick the over fox the\"", - "\"the quick brown fox over the lazy dog\"", "\"the quick brown fox over the very lazy dog\"", - "\"dog brown quick the over fox the lazy\"", + "\"the quick brown fox over the lazy dog\"", "\"the quick brown fox over\"", - "\"brown quick the over fox\"", "\"the very quick brown fox over\"", + "\"dog brown quick the over fox the lazy\"", + "\"brown quick the over fox\"", ] "###);