Fix term matching strategy bugs

This commit is contained in:
Loïc Lecrenier 2023-03-30 14:01:52 +02:00
parent 35c16ad047
commit d48cdc67a0
3 changed files with 7 additions and 8 deletions

View File

@ -107,9 +107,6 @@ impl<'ctx, G: RankingRuleGraphTrait> RankingRule<'ctx, QueryGraph> for GraphBase
query_graph: &QueryGraph, query_graph: &QueryGraph,
) -> Result<()> { ) -> Result<()> {
let removal_cost = if let Some(terms_matching_strategy) = self.terms_matching_strategy { let removal_cost = if let Some(terms_matching_strategy) = self.terms_matching_strategy {
// oh no this is wrong!
// because
// skipping the second node should require that the first one be skipped too
match terms_matching_strategy { match terms_matching_strategy {
TermsMatchingStrategy::Last => { TermsMatchingStrategy::Last => {
let removal_order = let removal_order =
@ -123,7 +120,7 @@ impl<'ctx, G: RankingRuleGraphTrait> RankingRule<'ctx, QueryGraph> for GraphBase
*costs.get_mut(n) = Some((cost, forbidden_nodes.clone())); *costs.get_mut(n) = Some((cost, forbidden_nodes.clone()));
} }
forbidden_nodes.union(&ns); forbidden_nodes.union(&ns);
cost = 1000; cost += 100;
} }
costs costs
} }

View File

@ -300,14 +300,14 @@ impl QueryGraph {
} }
}; };
let mut nodes_to_remove = BTreeMap::<u16, SmallBitmap<QueryNode>>::new(); let mut nodes_to_remove = BTreeMap::<u16, SmallBitmap<QueryNode>>::new();
for (node_id, node) in self.nodes.iter() { 'outer: for (node_id, node) in self.nodes.iter() {
let QueryNodeData::Term(t) = &node.data else { continue }; let QueryNodeData::Term(t) = &node.data else { continue };
let mut cost = 0; let mut cost = 0;
for id in t.term_ids.clone() { for id in t.term_ids.clone() {
if let Some(t_cost) = cost_of_term_idx(id) { if let Some(t_cost) = cost_of_term_idx(id) {
cost += t_cost; cost = std::cmp::max(cost, t_cost);
} else { } else {
continue; continue 'outer;
} }
} }
nodes_to_remove nodes_to_remove

View File

@ -40,7 +40,9 @@ impl<'ctx> RankingRule<'ctx, QueryGraph> for Words {
self.query_graph = Some(parent_query_graph.clone()); self.query_graph = Some(parent_query_graph.clone());
self.nodes_to_remove = match self.terms_matching_strategy { self.nodes_to_remove = match self.terms_matching_strategy {
TermsMatchingStrategy::Last => { TermsMatchingStrategy::Last => {
parent_query_graph.removal_order_for_terms_matching_strategy_last() let mut ns = parent_query_graph.removal_order_for_terms_matching_strategy_last();
ns.reverse();
ns
} }
TermsMatchingStrategy::All => { TermsMatchingStrategy::All => {
vec![] vec![]