From 2e896f30a50519d3c5435779dd2b1c41b66f158b Mon Sep 17 00:00:00 2001 From: ManyTheFish Date: Tue, 26 Nov 2024 15:53:54 +0100 Subject: [PATCH] Fix PR comments --- .../meilisearch/tests/search/facet_search.rs | 60 +++++++++---------- .../tests/settings/prefix_search_settings.rs | 30 +++++----- crates/milli/src/index.rs | 6 +- crates/milli/src/update/facet/mod.rs | 15 ++--- 4 files changed, 54 insertions(+), 57 deletions(-) diff --git a/crates/meilisearch/tests/search/facet_search.rs b/crates/meilisearch/tests/search/facet_search.rs index 52b8171c4..8fbeae293 100644 --- a/crates/meilisearch/tests/search/facet_search.rs +++ b/crates/meilisearch/tests/search/facet_search.rs @@ -41,8 +41,8 @@ async fn simple_facet_search() { let documents = DOCUMENTS.clone(); index.update_settings_filterable_attributes(json!(["genres"])).await; - index.add_documents(documents, None).await; - index.wait_task(1).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "a"})).await; @@ -65,8 +65,8 @@ async fn advanced_facet_search() { let documents = DOCUMENTS.clone(); index.update_settings_filterable_attributes(json!(["genres"])).await; index.update_settings_typo_tolerance(json!({ "enabled": false })).await; - index.add_documents(documents, None).await; - index.wait_task(2).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "adventre"})).await; @@ -89,8 +89,8 @@ async fn more_advanced_facet_search() { let documents = DOCUMENTS.clone(); index.update_settings_filterable_attributes(json!(["genres"])).await; index.update_settings_typo_tolerance(json!({ "disableOnWords": ["adventre"] })).await; - index.add_documents(documents, None).await; - index.wait_task(2).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "adventre"})).await; @@ -113,8 +113,8 @@ async fn simple_facet_search_with_max_values() { let documents = DOCUMENTS.clone(); index.update_settings_faceting(json!({ "maxValuesPerFacet": 1 })).await; index.update_settings_filterable_attributes(json!(["genres"])).await; - index.add_documents(documents, None).await; - index.wait_task(2).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "a"})).await; @@ -135,8 +135,8 @@ async fn simple_facet_search_by_count_with_max_values() { ) .await; index.update_settings_filterable_attributes(json!(["genres"])).await; - index.add_documents(documents, None).await; - index.wait_task(2).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "a"})).await; @@ -151,8 +151,8 @@ async fn non_filterable_facet_search_error() { let index = server.index("test"); let documents = DOCUMENTS.clone(); - index.add_documents(documents, None).await; - index.wait_task(0).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "a"})).await; @@ -170,8 +170,8 @@ async fn facet_search_dont_support_words() { let documents = DOCUMENTS.clone(); index.update_settings_filterable_attributes(json!(["genres"])).await; - index.add_documents(documents, None).await; - index.wait_task(1).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "words"})).await; @@ -188,8 +188,8 @@ async fn simple_facet_search_with_sort_by_count() { let documents = DOCUMENTS.clone(); index.update_settings_faceting(json!({ "sortFacetValuesBy": { "*": "count" } })).await; index.update_settings_filterable_attributes(json!(["genres"])).await; - index.add_documents(documents, None).await; - index.wait_task(2).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "a"})).await; @@ -207,8 +207,8 @@ async fn add_documents_and_deactivate_facet_search() { let index = server.index("test"); let documents = DOCUMENTS.clone(); - index.add_documents(documents, None).await; - index.wait_task(0).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index .update_settings(json!({ "facetSearch": false, @@ -216,7 +216,7 @@ async fn add_documents_and_deactivate_facet_search() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(1).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "a"})).await; @@ -237,10 +237,10 @@ async fn deactivate_facet_search_and_add_documents() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(0).await; + index.wait_task(response.uid()).await; let documents = DOCUMENTS.clone(); - index.add_documents(documents, None).await; - index.wait_task(1).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "a"})).await; @@ -261,10 +261,10 @@ async fn deactivate_facet_search_add_documents_and_activate_facet_search() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(0).await; + index.wait_task(response.uid()).await; let documents = DOCUMENTS.clone(); - index.add_documents(documents, None).await; - index.wait_task(1).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index .update_settings(json!({ @@ -272,7 +272,7 @@ async fn deactivate_facet_search_add_documents_and_activate_facet_search() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(2).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "a"})).await; @@ -293,10 +293,10 @@ async fn deactivate_facet_search_add_documents_and_reset_facet_search() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(0).await; + index.wait_task(response.uid()).await; let documents = DOCUMENTS.clone(); - index.add_documents(documents, None).await; - index.wait_task(1).await; + let (response, _code) = index.add_documents(documents, None).await; + index.wait_task(response.uid()).await; let (response, code) = index .update_settings(json!({ @@ -304,7 +304,7 @@ async fn deactivate_facet_search_add_documents_and_reset_facet_search() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(2).await; + index.wait_task(response.uid()).await; let (response, code) = index.facet_search(json!({"facetName": "genres", "facetQuery": "a"})).await; diff --git a/crates/meilisearch/tests/settings/prefix_search_settings.rs b/crates/meilisearch/tests/settings/prefix_search_settings.rs index 34a891f97..5da758a7d 100644 --- a/crates/meilisearch/tests/settings/prefix_search_settings.rs +++ b/crates/meilisearch/tests/settings/prefix_search_settings.rs @@ -29,8 +29,8 @@ async fn add_docs_and_disable() { let server = Server::new().await; let index = server.index("test"); - index.add_documents(DOCUMENTS.clone(), None).await; - index.wait_task(0).await; + let (response, _code) = index.add_documents(DOCUMENTS.clone(), None).await; + index.wait_task(response.uid()).await; let (response, code) = index .update_settings(json!({ @@ -39,7 +39,7 @@ async fn add_docs_and_disable() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(1).await; + index.wait_task(response.uid()).await; // only 1 document should match index @@ -96,10 +96,10 @@ async fn disable_and_add_docs() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(0).await; + index.wait_task(response.uid()).await; - index.add_documents(DOCUMENTS.clone(), None).await; - index.wait_task(1).await; + let (response, _code) = index.add_documents(DOCUMENTS.clone(), None).await; + index.wait_task(response.uid()).await; // only 1 document should match index @@ -155,10 +155,10 @@ async fn disable_add_docs_and_enable() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(0).await; + index.wait_task(response.uid()).await; - index.add_documents(DOCUMENTS.clone(), None).await; - index.wait_task(1).await; + let (response, _code) = index.add_documents(DOCUMENTS.clone(), None).await; + index.wait_task(response.uid()).await; let (response, code) = index .update_settings(json!({ @@ -263,10 +263,10 @@ async fn disable_add_docs_and_reset() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(0).await; + index.wait_task(response.uid()).await; - index.add_documents(DOCUMENTS.clone(), None).await; - index.wait_task(1).await; + let (response, _code) = index.add_documents(DOCUMENTS.clone(), None).await; + index.wait_task(response.uid()).await; let (response, code) = index .update_settings(json!({ @@ -370,10 +370,10 @@ async fn default_behavior() { })) .await; assert_eq!("202", code.as_str(), "{:?}", response); - index.wait_task(0).await; + index.wait_task(response.uid()).await; - index.add_documents(DOCUMENTS.clone(), None).await; - index.wait_task(1).await; + let (response, _code) = index.add_documents(DOCUMENTS.clone(), None).await; + index.wait_task(response.uid()).await; // all documents should match index diff --git a/crates/milli/src/index.rs b/crates/milli/src/index.rs index b2f3cdbd1..fe83877a7 100644 --- a/crates/milli/src/index.rs +++ b/crates/milli/src/index.rs @@ -1690,11 +1690,7 @@ impl Index { pub fn prefix_settings(&self, rtxn: &RoTxn<'_>) -> Result { let compute_prefixes = self.prefix_search(rtxn)?.unwrap_or_default(); - Ok(PrefixSettings { - compute_prefixes, - max_prefix_length: 4, - prefix_count_threshold: 100, - }) + Ok(PrefixSettings { compute_prefixes, max_prefix_length: 4, prefix_count_threshold: 100 }) } } diff --git a/crates/milli/src/update/facet/mod.rs b/crates/milli/src/update/facet/mod.rs index 3eaf2f221..911296577 100644 --- a/crates/milli/src/update/facet/mod.rs +++ b/crates/milli/src/update/facet/mod.rs @@ -172,14 +172,15 @@ impl<'i> FacetsUpdate<'i> { incremental_update.execute(wtxn)?; } + if !self.index.facet_search(wtxn)? { + // If facet search is disabled, we don't need to compute facet search databases. + // We clear the facet search databases. + self.index.facet_id_string_fst.clear(wtxn)?; + self.index.facet_id_normalized_string_strings.clear(wtxn)?; + return Ok(()); + } + match self.normalized_delta_data { - _ if !self.index.facet_search(wtxn)? => { - // If facet search is disabled, we don't need to compute facet search databases. - // We clear the facet search databases. - self.index.facet_id_string_fst.clear(wtxn)?; - self.index.facet_id_normalized_string_strings.clear(wtxn)?; - Ok(()) - } Some(data) => index_facet_search(wtxn, data, self.index), None => Ok(()), }