From ed1dcbe0f78fc7350dd8c461dcfef2f56499f612 Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Thu, 6 Mar 2025 14:18:25 +0100 Subject: [PATCH] Fix behavior change in the Attributes criterion --- crates/milli/src/index.rs | 11 ++++ .../search/new/ranking_rule_graph/fid/mod.rs | 5 +- ...__attribute_position_different_fields.snap | 56 +++++++++---------- ...e_position__attribute_position_ngrams.snap | 56 +++++++++---------- ...position__attribute_position_repeated.snap | 20 +++---- ...position__attribute_position_simple-2.snap | 56 +++++++++---------- 6 files changed, 109 insertions(+), 95 deletions(-) diff --git a/crates/milli/src/index.rs b/crates/milli/src/index.rs index f9109a137..798cf3073 100644 --- a/crates/milli/src/index.rs +++ b/crates/milli/src/index.rs @@ -559,6 +559,17 @@ impl Index { self.main.remap_key_type::().delete(wtxn, main_key::FIELDIDS_WEIGHTS_MAP_KEY) } + pub fn max_searchable_attribute_weight(&self, rtxn: &RoTxn<'_>) -> Result> { + let user_defined_searchable_fields = self.user_defined_searchable_fields(rtxn)?; + if let Some(user_defined_searchable_fields) = user_defined_searchable_fields { + if !user_defined_searchable_fields.contains(&"*") { + return Ok(Some(user_defined_searchable_fields.len().saturating_sub(1) as Weight)); + } + } + + Ok(None) + } + pub fn searchable_fields_and_weights<'a>( &self, rtxn: &'a RoTxn<'a>, diff --git a/crates/milli/src/search/new/ranking_rule_graph/fid/mod.rs b/crates/milli/src/search/new/ranking_rule_graph/fid/mod.rs index f424b7241..e55f1febf 100644 --- a/crates/milli/src/search/new/ranking_rule_graph/fid/mod.rs +++ b/crates/milli/src/search/new/ranking_rule_graph/fid/mod.rs @@ -92,7 +92,10 @@ impl RankingRuleGraphTrait for FidGraph { } // always lookup the max_fid if we don't already and add an artificial condition for max scoring - let max_weight = weights_map.max_weight(); + let max_weight = ctx + .index + .max_searchable_attribute_weight(ctx.txn)? + .or_else(|| weights_map.max_weight()); if let Some(max_weight) = max_weight { if current_max_weight < max_weight { diff --git a/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_different_fields.snap b/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_different_fields.snap index bf5b14f47..e55e221a2 100644 --- a/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_different_fields.snap +++ b/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_different_fields.snap @@ -8,8 +8,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -25,8 +25,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -42,8 +42,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -59,8 +59,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -76,8 +76,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -93,8 +93,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -110,8 +110,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -127,8 +127,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -144,8 +144,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -161,8 +161,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -178,8 +178,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -195,8 +195,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -212,8 +212,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -229,8 +229,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( diff --git a/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_ngrams.snap b/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_ngrams.snap index bf5b14f47..e55e221a2 100644 --- a/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_ngrams.snap +++ b/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_ngrams.snap @@ -8,8 +8,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -25,8 +25,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -42,8 +42,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -59,8 +59,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -76,8 +76,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -93,8 +93,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -110,8 +110,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -127,8 +127,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -144,8 +144,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -161,8 +161,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -178,8 +178,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -195,8 +195,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -212,8 +212,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -229,8 +229,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( diff --git a/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_repeated.snap b/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_repeated.snap index af35d0d8d..5ae6fafc5 100644 --- a/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_repeated.snap +++ b/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_repeated.snap @@ -8,8 +8,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 6, - max_rank: 6, + rank: 11, + max_rank: 11, }, ), Position( @@ -25,8 +25,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 6, - max_rank: 6, + rank: 11, + max_rank: 11, }, ), Position( @@ -42,8 +42,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 6, - max_rank: 6, + rank: 11, + max_rank: 11, }, ), Position( @@ -59,8 +59,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 6, - max_rank: 6, + rank: 11, + max_rank: 11, }, ), Position( @@ -76,8 +76,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 6, - max_rank: 6, + rank: 11, + max_rank: 11, }, ), Position( diff --git a/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_simple-2.snap b/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_simple-2.snap index bf5b14f47..e55e221a2 100644 --- a/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_simple-2.snap +++ b/crates/milli/src/search/new/tests/snapshots/milli__search__new__tests__attribute_position__attribute_position_simple-2.snap @@ -8,8 +8,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -25,8 +25,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -42,8 +42,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -59,8 +59,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -76,8 +76,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -93,8 +93,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -110,8 +110,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -127,8 +127,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -144,8 +144,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -161,8 +161,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -178,8 +178,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -195,8 +195,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -212,8 +212,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position( @@ -229,8 +229,8 @@ expression: "format!(\"{document_ids_scores:#?}\")" [ Fid( Rank { - rank: 3, - max_rank: 3, + rank: 5, + max_rank: 5, }, ), Position(