From e44325683abc93809eaa8c5ea6cdcfb658248e12 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 12 Sep 2024 17:40:33 +0200 Subject: [PATCH 01/26] Facet distribution: fix issue where truncated facet distribution would have a wrong order --- milli/src/search/facet/facet_distribution.rs | 23 +++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/milli/src/search/facet/facet_distribution.rs b/milli/src/search/facet/facet_distribution.rs index 62ae05740..fb1a255f3 100644 --- a/milli/src/search/facet/facet_distribution.rs +++ b/milli/src/search/facet/facet_distribution.rs @@ -100,7 +100,6 @@ impl<'a> FacetDistribution<'a> { let mut lexicographic_distribution = BTreeMap::new(); let mut key_buffer: Vec<_> = field_id.to_be_bytes().to_vec(); - let distribution_prelength = distribution.len(); let db = self.index.field_id_docid_facet_f64s; for docid in candidates { key_buffer.truncate(mem::size_of::()); @@ -113,23 +112,21 @@ impl<'a> FacetDistribution<'a> { for result in iter { let ((_, _, value), ()) = result?; *lexicographic_distribution.entry(value.to_string()).or_insert(0) += 1; - - if lexicographic_distribution.len() - distribution_prelength - == self.max_values_per_facet - { - break; - } } } - distribution.extend(lexicographic_distribution); + distribution.extend( + lexicographic_distribution + .into_iter() + .take(self.max_values_per_facet.saturating_sub(distribution.len())), + ); } FacetType::String => { let mut normalized_distribution = BTreeMap::new(); let mut key_buffer: Vec<_> = field_id.to_be_bytes().to_vec(); let db = self.index.field_id_docid_facet_strings; - 'outer: for docid in candidates { + for docid in candidates { key_buffer.truncate(mem::size_of::()); key_buffer.extend_from_slice(&docid.to_be_bytes()); let iter = db @@ -144,14 +141,14 @@ impl<'a> FacetDistribution<'a> { .or_insert_with(|| (original_value, 0)); *count += 1; - if normalized_distribution.len() == self.max_values_per_facet { - break 'outer; - } + // we'd like to break here if we have enough facet values, but we are collecting them by increasing docid, + // so higher ranked facets could be in later docids } } let iter = normalized_distribution .into_iter() + .take(self.max_values_per_facet.saturating_sub(distribution.len())) .map(|(_normalized, (original, count))| (original.to_string(), count)); distribution.extend(iter); } @@ -467,7 +464,7 @@ mod tests { .execute() .unwrap(); - milli_snap!(format!("{map:?}"), @r###"{"colour": {"Blue": 1}}"###); + milli_snap!(format!("{map:?}"), @r###"{"colour": {"Blue": 2}}"###); let map = FacetDistribution::new(&txn, &index) .facets(iter::once(("colour", OrderBy::Count))) From 23e14138bbb6d62d3e0a8745d538b6b7ac90d8b2 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 12 Sep 2024 17:41:01 +0200 Subject: [PATCH 02/26] facet distribution: implement Display for OrderBy --- milli/src/search/facet/facet_distribution.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/milli/src/search/facet/facet_distribution.rs b/milli/src/search/facet/facet_distribution.rs index fb1a255f3..a63bb634b 100644 --- a/milli/src/search/facet/facet_distribution.rs +++ b/milli/src/search/facet/facet_distribution.rs @@ -1,4 +1,5 @@ use std::collections::{BTreeMap, HashMap, HashSet}; +use std::fmt::Display; use std::ops::ControlFlow; use std::{fmt, mem}; @@ -37,6 +38,15 @@ pub enum OrderBy { Count, } +impl Display for OrderBy { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + OrderBy::Lexicographic => f.write_str("alphabetically"), + OrderBy::Count => f.write_str("by count"), + } + } +} + pub struct FacetDistribution<'a> { facets: Option>, candidates: Option, From a94a87ee5417816db870c1aeb542e0ad37074890 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Wed, 11 Sep 2024 11:25:26 +0200 Subject: [PATCH 03/26] Slightly changes existing error messages --- meilisearch/src/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/meilisearch/src/error.rs b/meilisearch/src/error.rs index 41473245e..c7b109598 100644 --- a/meilisearch/src/error.rs +++ b/meilisearch/src/error.rs @@ -27,9 +27,9 @@ pub enum MeilisearchHttpError { EmptyFilter, #[error("Invalid syntax for the filter parameter: `expected {}, found: {1}`.", .0.join(", "))] InvalidExpression(&'static [&'static str], Value), - #[error("Using `federationOptions` is not allowed in a non-federated search.\n Hint: remove `federationOptions` from query #{0} or add `federation: {{}}` to the request.")] + #[error("Using `federationOptions` is not allowed in a non-federated search.\n - Hint: remove `federationOptions` from query #{0} or add `federation` to the request.")] FederationOptionsInNonFederatedRequest(usize), - #[error("Inside `.queries[{0}]`: Using pagination options is not allowed in federated queries.\n Hint: remove `{1}` from query #{0} or remove `federation: {{}}` from the request")] + #[error("Inside `.queries[{0}]`: Using pagination options is not allowed in federated queries.\n - Hint: remove `{1}` from query #{0} or remove `federation` from the request\n - Hint: pass `federation.limit` and `federation.offset` for pagination in federated search")] PaginationInFederatedQuery(usize, &'static str), #[error("A {0} payload is missing.")] MissingPayload(PayloadType), From a48b1d5a791406113964799c4908c775f85b551d Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 12 Sep 2024 17:51:54 +0200 Subject: [PATCH 04/26] Update existing tests following error message changes --- meilisearch/tests/search/multi.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/meilisearch/tests/search/multi.rs b/meilisearch/tests/search/multi.rs index 08ad0b18c..f92b9bfc8 100644 --- a/meilisearch/tests/search/multi.rs +++ b/meilisearch/tests/search/multi.rs @@ -3799,7 +3799,7 @@ async fn federation_federated_contains_pagination() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.queries[1]`: Using pagination options is not allowed in federated queries.\n Hint: remove `limit` from query #1 or remove `federation: {}` from the request", + "message": "Inside `.queries[1]`: Using pagination options is not allowed in federated queries.\n - Hint: remove `limit` from query #1 or remove `federation` from the request\n - Hint: pass `federation.limit` and `federation.offset` for pagination in federated search", "code": "invalid_multi_search_query_pagination", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_query_pagination" @@ -3815,7 +3815,7 @@ async fn federation_federated_contains_pagination() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.queries[1]`: Using pagination options is not allowed in federated queries.\n Hint: remove `offset` from query #1 or remove `federation: {}` from the request", + "message": "Inside `.queries[1]`: Using pagination options is not allowed in federated queries.\n - Hint: remove `offset` from query #1 or remove `federation` from the request\n - Hint: pass `federation.limit` and `federation.offset` for pagination in federated search", "code": "invalid_multi_search_query_pagination", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_query_pagination" @@ -3831,7 +3831,7 @@ async fn federation_federated_contains_pagination() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.queries[1]`: Using pagination options is not allowed in federated queries.\n Hint: remove `page` from query #1 or remove `federation: {}` from the request", + "message": "Inside `.queries[1]`: Using pagination options is not allowed in federated queries.\n - Hint: remove `page` from query #1 or remove `federation` from the request\n - Hint: pass `federation.limit` and `federation.offset` for pagination in federated search", "code": "invalid_multi_search_query_pagination", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_query_pagination" @@ -3847,7 +3847,7 @@ async fn federation_federated_contains_pagination() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.queries[1]`: Using pagination options is not allowed in federated queries.\n Hint: remove `hitsPerPage` from query #1 or remove `federation: {}` from the request", + "message": "Inside `.queries[1]`: Using pagination options is not allowed in federated queries.\n - Hint: remove `hitsPerPage` from query #1 or remove `federation` from the request\n - Hint: pass `federation.limit` and `federation.offset` for pagination in federated search", "code": "invalid_multi_search_query_pagination", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_query_pagination" @@ -3875,7 +3875,7 @@ async fn federation_non_federated_contains_federation_option() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.queries[1]`: Using `federationOptions` is not allowed in a non-federated search.\n Hint: remove `federationOptions` from query #1 or add `federation: {}` to the request.", + "message": "Inside `.queries[1]`: Using `federationOptions` is not allowed in a non-federated search.\n - Hint: remove `federationOptions` from query #1 or add `federation` to the request.", "code": "invalid_multi_search_federation_options", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_federation_options" From 72cc573e0acb2937e2ee66a034291a2f0a5caeb7 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 12 Sep 2024 17:47:00 +0200 Subject: [PATCH 05/26] Add new error types --- meilisearch-types/src/error.rs | 6 ++++++ meilisearch/src/error.rs | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/meilisearch-types/src/error.rs b/meilisearch-types/src/error.rs index 0099cada5..bf89fe614 100644 --- a/meilisearch-types/src/error.rs +++ b/meilisearch-types/src/error.rs @@ -238,10 +238,16 @@ InvalidIndexLimit , InvalidRequest , BAD_REQUEST ; InvalidIndexOffset , InvalidRequest , BAD_REQUEST ; InvalidIndexPrimaryKey , InvalidRequest , BAD_REQUEST ; InvalidIndexUid , InvalidRequest , BAD_REQUEST ; +InvalidMultiSearchFacetsByIndex , InvalidRequest , BAD_REQUEST ; +InvalidMultiSearchFacetOrder , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchFederated , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchFederationOptions , InvalidRequest , BAD_REQUEST ; +InvalidMultiSearchMaxValuesPerFacet , InvalidRequest , BAD_REQUEST ; +InvalidMultiSearchMergeFacets , InvalidRequest , BAD_REQUEST ; +InvalidMultiSearchQueryFacets , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchQueryPagination , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchQueryRankingRules , InvalidRequest , BAD_REQUEST ; +InvalidMultiSearchSortFacetValuesBy , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchWeight , InvalidRequest , BAD_REQUEST ; InvalidSearchAttributesToSearchOn , InvalidRequest , BAD_REQUEST ; InvalidSearchAttributesToCrop , InvalidRequest , BAD_REQUEST ; diff --git a/meilisearch/src/error.rs b/meilisearch/src/error.rs index c7b109598..fa315837f 100644 --- a/meilisearch/src/error.rs +++ b/meilisearch/src/error.rs @@ -4,6 +4,7 @@ use byte_unit::{Byte, UnitType}; use meilisearch_types::document_formats::{DocumentFormatError, PayloadType}; use meilisearch_types::error::{Code, ErrorCode, ResponseError}; use meilisearch_types::index_uid::{IndexUid, IndexUidFormatError}; +use meilisearch_types::milli::OrderBy; use serde_json::Value; use tokio::task::JoinError; @@ -31,6 +32,16 @@ pub enum MeilisearchHttpError { FederationOptionsInNonFederatedRequest(usize), #[error("Inside `.queries[{0}]`: Using pagination options is not allowed in federated queries.\n - Hint: remove `{1}` from query #{0} or remove `federation` from the request\n - Hint: pass `federation.limit` and `federation.offset` for pagination in federated search")] PaginationInFederatedQuery(usize, &'static str), + #[error("Inside `.queries[{0}]`: Using facet options is not allowed in federated queries.\n Hint: remove `facets` from query #{0} or remove `federation` from the request")] + FacetsInFederatedQuery(usize), + #[error("Inconsistent order for values in facet `{facet}`: index `{previous_uid}` orders {previous_facet_order}, but index `{current_uid}` orders {index_facet_order}.\n Hint: Remove `federation.mergeFacets` or set `federation.mergeFacets.sortFacetValuesBy` to the desired order.")] + InconsistentFacetOrder { + facet: String, + previous_facet_order: OrderBy, + previous_uid: String, + index_facet_order: OrderBy, + current_uid: String, + }, #[error("A {0} payload is missing.")] MissingPayload(PayloadType), #[error("Too many search requests running at the same time: {0}. Retry after 10s.")] @@ -96,6 +107,10 @@ impl ErrorCode for MeilisearchHttpError { MeilisearchHttpError::PaginationInFederatedQuery(_, _) => { Code::InvalidMultiSearchQueryPagination } + MeilisearchHttpError::FacetsInFederatedQuery(_) => Code::InvalidMultiSearchQueryFacets, + MeilisearchHttpError::InconsistentFacetOrder { .. } => { + Code::InvalidMultiSearchFacetOrder + } } } } From 57f9517a987579a2ef8759e182622502afcaa353 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 12 Sep 2024 17:47:15 +0200 Subject: [PATCH 06/26] Required changes to IndexUid --- meilisearch-types/src/index_uid.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/meilisearch-types/src/index_uid.rs b/meilisearch-types/src/index_uid.rs index 341ab02cb..d64a6658d 100644 --- a/meilisearch-types/src/index_uid.rs +++ b/meilisearch-types/src/index_uid.rs @@ -1,3 +1,4 @@ +use std::borrow::Borrow; use std::error::Error; use std::fmt; use std::str::FromStr; @@ -8,7 +9,7 @@ use crate::error::{Code, ErrorCode}; /// An index uid is composed of only ascii alphanumeric characters, - and _, between 1 and 400 /// bytes long -#[derive(Debug, Clone, PartialEq, Eq, Deserr)] +#[derive(Debug, Clone, PartialEq, Eq, Deserr, PartialOrd, Ord)] #[deserr(try_from(String) = IndexUid::try_from -> IndexUidFormatError)] pub struct IndexUid(String); @@ -70,6 +71,12 @@ impl From for String { } } +impl Borrow for IndexUid { + fn borrow(&self) -> &String { + &self.0 + } +} + #[derive(Debug)] pub struct IndexUidFormatError { pub invalid_uid: String, From 7c084b1286d6b7374d6d034181a418059177c253 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 12 Sep 2024 17:48:26 +0200 Subject: [PATCH 07/26] SearchQueriesWithIndex changes --- meilisearch/src/search/mod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/meilisearch/src/search/mod.rs b/meilisearch/src/search/mod.rs index 915505be0..e8e1fec37 100644 --- a/meilisearch/src/search/mod.rs +++ b/meilisearch/src/search/mod.rs @@ -441,9 +441,6 @@ pub struct SearchQueryWithIndex { } impl SearchQueryWithIndex { - pub fn has_federation_options(&self) -> bool { - self.federation_options.is_some() - } pub fn has_pagination(&self) -> Option<&'static str> { if self.offset.is_some() { Some("offset") @@ -458,6 +455,11 @@ impl SearchQueryWithIndex { } } + pub fn has_facets(&self) -> bool { + let Some(facets) = &self.facets else { return false }; + !facets.is_empty() + } + pub fn into_index_query_federation(self) -> (IndexUid, SearchQuery, Option) { let SearchQueryWithIndex { index_uid, From f6114a1ff263655339060e395b5865c2bed11940 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 12 Sep 2024 17:49:03 +0200 Subject: [PATCH 08/26] Introduce ComputedFacets and compute_facet_distribution_stats --- meilisearch/src/search/mod.rs | 89 ++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 33 deletions(-) diff --git a/meilisearch/src/search/mod.rs b/meilisearch/src/search/mod.rs index e8e1fec37..99245bdc1 100644 --- a/meilisearch/src/search/mod.rs +++ b/meilisearch/src/search/mod.rs @@ -989,39 +989,13 @@ pub fn perform_search( HitsInfo::OffsetLimit { limit, offset, estimated_total_hits: number_of_hits } }; - let (facet_distribution, facet_stats) = match facets { - Some(ref fields) => { - let mut facet_distribution = index.facets_distribution(&rtxn); - - let max_values_by_facet = index - .max_values_per_facet(&rtxn) - .map_err(milli::Error::from)? - .map(|x| x as usize) - .unwrap_or(DEFAULT_VALUES_PER_FACET); - facet_distribution.max_values_per_facet(max_values_by_facet); - - let sort_facet_values_by = - index.sort_facet_values_by(&rtxn).map_err(milli::Error::from)?; - - if fields.iter().all(|f| f != "*") { - let fields: Vec<_> = - fields.iter().map(|n| (n, sort_facet_values_by.get(n))).collect(); - facet_distribution.facets(fields); - } - - let distribution = facet_distribution - .candidates(candidates) - .default_order_by(sort_facet_values_by.get("*")) - .execute()?; - let stats = facet_distribution.compute_stats()?; - (Some(distribution), Some(stats)) - } - None => (None, None), - }; - - let facet_stats = facet_stats.map(|stats| { - stats.into_iter().map(|(k, (min, max))| (k, FacetStats { min, max })).collect() - }); + let (facet_distribution, facet_stats) = facets + .map(move |facets| { + compute_facet_distribution_stats(&facets, index, &rtxn, candidates, None, None) + }) + .transpose()? + .map(|ComputedFacets { distribution, stats }| (distribution, stats)) + .unzip(); let result = SearchResult { hits: documents, @@ -1037,6 +1011,55 @@ pub fn perform_search( Ok(result) } +#[derive(Debug, Clone, Default, Serialize)] +pub struct ComputedFacets { + pub distribution: BTreeMap>, + pub stats: BTreeMap, +} + +fn compute_facet_distribution_stats>( + facets: &[S], + index: &Index, + rtxn: &RoTxn, + candidates: roaring::RoaringBitmap, + override_max_values_per_facet: Option, + override_sort_facet_values_by: Option, +) -> Result { + let mut facet_distribution = index.facets_distribution(rtxn); + + let max_values_by_facet = match override_max_values_per_facet { + Some(max_values_by_facet) => max_values_by_facet, + None => index + .max_values_per_facet(rtxn) + .map_err(milli::Error::from)? + .map(|x| x as usize) + .unwrap_or(DEFAULT_VALUES_PER_FACET), + }; + + facet_distribution.max_values_per_facet(max_values_by_facet); + + let sort_facet_values_by = index.sort_facet_values_by(rtxn).map_err(milli::Error::from)?; + + let sort_facet_values_by = |n: &str| match override_sort_facet_values_by { + Some(order_by) => order_by, + None => sort_facet_values_by.get(n), + }; + + // add specific facet if there is no placeholder + if facets.iter().all(|f| f.as_ref() != "*") { + let fields: Vec<_> = facets.iter().map(|n| (n, sort_facet_values_by(n.as_ref()))).collect(); + facet_distribution.facets(fields); + } + + let distribution = facet_distribution + .candidates(candidates) + .default_order_by(sort_facet_values_by("*")) + .execute()?; + let stats = facet_distribution.compute_stats()?; + let stats = stats.into_iter().map(|(k, (min, max))| (k, FacetStats { min, max })).collect(); + Ok(ComputedFacets { distribution, stats }) +} + pub fn search_from_kind( search_kind: SearchKind, search: milli::Search<'_>, From 7b55462610d0bfe3a07c0eab6d57c71803619db4 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 12 Sep 2024 17:50:03 +0200 Subject: [PATCH 09/26] BREAKING CHANGE: errors if queries.facets in federated search --- meilisearch/src/search/federated.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/meilisearch/src/search/federated.rs b/meilisearch/src/search/federated.rs index 58005ec53..f1acf5aa4 100644 --- a/meilisearch/src/search/federated.rs +++ b/meilisearch/src/search/federated.rs @@ -342,6 +342,10 @@ pub fn perform_federated_search( .into()); } + if federated_query.has_facets() { + return Err(MeilisearchHttpError::FacetsInFederatedQuery(query_index).into()); + } + let (index_uid, query, federation_options) = federated_query.into_index_query_federation(); queries_by_index.entry(index_uid.into_inner()).or_default().push(QueryByIndex { From 533f1d4345d9069f048ecd50555ec83ad213f87b Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 12 Sep 2024 17:51:20 +0200 Subject: [PATCH 10/26] Federated search: support facets --- meilisearch/src/search/federated.rs | 363 ++++++++++++++++++++++++++-- 1 file changed, 347 insertions(+), 16 deletions(-) diff --git a/meilisearch/src/search/federated.rs b/meilisearch/src/search/federated.rs index f1acf5aa4..9d16ca59d 100644 --- a/meilisearch/src/search/federated.rs +++ b/meilisearch/src/search/federated.rs @@ -9,20 +9,24 @@ use std::vec::{IntoIter, Vec}; use actix_http::StatusCode; use index_scheduler::{IndexScheduler, RoFeatures}; +use indexmap::IndexMap; use meilisearch_types::deserr::DeserrJsonError; use meilisearch_types::error::deserr_codes::{ - InvalidMultiSearchWeight, InvalidSearchLimit, InvalidSearchOffset, + InvalidMultiSearchFacetsByIndex, InvalidMultiSearchMaxValuesPerFacet, + InvalidMultiSearchMergeFacets, InvalidMultiSearchSortFacetValuesBy, InvalidMultiSearchWeight, + InvalidSearchLimit, InvalidSearchOffset, }; use meilisearch_types::error::ResponseError; +use meilisearch_types::index_uid::IndexUid; use meilisearch_types::milli::score_details::{ScoreDetails, ScoreValue}; -use meilisearch_types::milli::{self, DocumentId, TimeBudget}; +use meilisearch_types::milli::{self, DocumentId, OrderBy, TimeBudget}; use roaring::RoaringBitmap; use serde::Serialize; use super::ranking_rules::{self, RankingRules}; use super::{ - prepare_search, AttributesFormat, HitMaker, HitsInfo, RetrieveVectors, SearchHit, SearchKind, - SearchQuery, SearchQueryWithIndex, + compute_facet_distribution_stats, prepare_search, AttributesFormat, ComputedFacets, FacetStats, + HitMaker, HitsInfo, RetrieveVectors, SearchHit, SearchKind, SearchQuery, SearchQueryWithIndex, }; use crate::error::MeilisearchHttpError; use crate::routes::indexes::search::search_kind; @@ -73,6 +77,59 @@ pub struct Federation { pub limit: usize, #[deserr(default = super::DEFAULT_SEARCH_OFFSET(), error = DeserrJsonError)] pub offset: usize, + #[deserr(default, error = DeserrJsonError)] + pub facets_by_index: BTreeMap>>, + #[deserr(default, error = DeserrJsonError)] + pub merge_facets: Option, +} + +#[derive(Copy, Clone, Debug, deserr::Deserr, Default)] +#[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] +pub struct MergeFacets { + #[deserr(default, error = DeserrJsonError)] + pub sort_facet_values_by: SortFacetValuesBy, + #[deserr(default, error = DeserrJsonError)] + pub max_values_per_facet: Option, +} + +impl MergeFacets { + pub fn to_components(this: Option) -> (Option, Option) { + match this { + Some(MergeFacets { sort_facet_values_by, max_values_per_facet }) => { + (sort_facet_values_by.into(), max_values_per_facet) + } + None => (None, None), + } + } +} + +#[derive(Debug, deserr::Deserr, Default, Clone, Copy)] +#[deserr(rename_all = camelCase, deny_unknown_fields)] +pub enum SortFacetValuesBy { + #[default] + IndexSettings, + /// By lexicographic order... + Alpha, + /// Or by number of docids in common? + Count, +} + +impl From for Option { + fn from(value: SortFacetValuesBy) -> Self { + match value { + SortFacetValuesBy::Alpha => Some(OrderBy::Lexicographic), + SortFacetValuesBy::Count => Some(OrderBy::Count), + SortFacetValuesBy::IndexSettings => None, + } + } +} + +#[derive(Debug, deserr::Deserr, Default)] +#[deserr(rename_all = camelCase, deny_unknown_fields)] +pub enum GroupFacetsBy { + Facet, + #[default] + Index, } #[derive(Debug, deserr::Deserr)] @@ -82,7 +139,7 @@ pub struct FederatedSearch { #[deserr(default)] pub federation: Option, } -#[derive(Serialize, Clone, PartialEq)] +#[derive(Serialize, Clone)] #[serde(rename_all = "camelCase")] pub struct FederatedSearchResult { pub hits: Vec, @@ -93,6 +150,13 @@ pub struct FederatedSearchResult { #[serde(skip_serializing_if = "Option::is_none")] pub semantic_hit_count: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub facet_distribution: Option>>, + #[serde(skip_serializing_if = "Option::is_none")] + pub facet_stats: Option>, + #[serde(skip_serializing_if = "FederatedFacets::is_empty")] + pub facets_by_index: FederatedFacets, + // These fields are only used for analytics purposes #[serde(skip)] pub degraded: bool, @@ -109,6 +173,9 @@ impl fmt::Debug for FederatedSearchResult { semantic_hit_count, degraded, used_negative_operator, + facet_distribution, + facet_stats, + facets_by_index, } = self; let mut debug = f.debug_struct("SearchResult"); @@ -122,9 +189,18 @@ impl fmt::Debug for FederatedSearchResult { if *degraded { debug.field("degraded", degraded); } + if let Some(facet_distribution) = facet_distribution { + debug.field("facet_distribution", &facet_distribution); + } + if let Some(facet_stats) = facet_stats { + debug.field("facet_stats", &facet_stats); + } if let Some(semantic_hit_count) = semantic_hit_count { debug.field("semantic_hit_count", &semantic_hit_count); } + if !facets_by_index.is_empty() { + debug.field("facets_by_index", &facets_by_index); + } debug.finish() } @@ -313,16 +389,111 @@ struct SearchHitByIndex { } struct SearchResultByIndex { + index: String, hits: Vec, - candidates: RoaringBitmap, + estimated_total_hits: usize, degraded: bool, used_negative_operator: bool, + facets: Option, +} + +#[derive(Debug, Clone, Default, Serialize)] +pub struct FederatedFacets(pub BTreeMap); + +impl FederatedFacets { + pub fn insert(&mut self, index: String, facets: Option) { + if let Some(facets) = facets { + self.0.insert(index, facets); + } + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + pub fn merge( + self, + MergeFacets { sort_facet_values_by, max_values_per_facet }: MergeFacets, + facet_order: Option>, + ) -> Option { + if self.is_empty() { + return None; + } + + let mut distribution: BTreeMap = Default::default(); + let mut stats: BTreeMap = Default::default(); + + for facets_by_index in self.0.into_values() { + for (facet, index_distribution) in facets_by_index.distribution { + match distribution.entry(facet) { + std::collections::btree_map::Entry::Vacant(entry) => { + entry.insert(index_distribution); + } + std::collections::btree_map::Entry::Occupied(mut entry) => { + let distribution = entry.get_mut(); + + for (value, index_count) in index_distribution { + distribution + .entry(value) + .and_modify(|count| *count += index_count) + .or_insert(index_count); + } + } + } + } + + for (facet, index_stats) in facets_by_index.stats { + match stats.entry(facet) { + std::collections::btree_map::Entry::Vacant(entry) => { + entry.insert(index_stats); + } + std::collections::btree_map::Entry::Occupied(mut entry) => { + let stats = entry.get_mut(); + + stats.min = + if stats.min <= index_stats.min { stats.min } else { index_stats.min }; + stats.max = + if stats.max >= index_stats.max { stats.max } else { index_stats.max }; + } + } + } + } + + // fixup order + for (facet, values) in &mut distribution { + let order_by = Option::::from(sort_facet_values_by) + .or_else(|| match &facet_order { + Some(facet_order) => facet_order.get(facet).map(|(_, order)| *order), + None => None, + }) + .unwrap_or_default(); + + match order_by { + OrderBy::Lexicographic => { + values.sort_unstable_by(|left, _, right, _| left.cmp(right)) + } + OrderBy::Count => { + values.sort_unstable_by(|_, left, _, right| { + left.cmp(right) + // biggest first + .reverse() + }) + } + } + + if let Some(max_values_per_facet) = max_values_per_facet { + values.truncate(max_values_per_facet) + }; + } + + Some(ComputedFacets { distribution, stats }) + } } pub fn perform_federated_search( index_scheduler: &IndexScheduler, queries: Vec, - federation: Federation, + mut federation: Federation, features: RoFeatures, ) -> Result { let before_search = std::time::Instant::now(); @@ -357,13 +528,29 @@ pub fn perform_federated_search( // 2. perform queries, merge and make hits index by index let required_hit_count = federation.limit + federation.offset; + + let (override_sort_facet_values_by, override_max_values_per_facet) = + MergeFacets::to_components(federation.merge_facets); + // In step (2), semantic_hit_count will be set to Some(0) if any search kind uses semantic // Then in step (3), we'll update its value if there is any semantic search let mut semantic_hit_count = None; let mut results_by_index = Vec::with_capacity(queries_by_index.len()); let mut previous_query_data: Option<(RankingRules, usize, String)> = None; + // remember the order and name of first index for each facet when merging with index settings + // to detect if the order is inconsistent for a facet. + let mut facet_order: Option> = match federation.merge_facets + { + Some(MergeFacets { sort_facet_values_by: SortFacetValuesBy::IndexSettings, .. }) => { + Some(Default::default()) + } + _ => None, + }; + for (index_uid, queries) in queries_by_index { + let first_query_index = queries.first().map(|query| query.query_index); + let index = match index_scheduler.index(&index_uid) { Ok(index) => index, Err(err) => { @@ -371,9 +558,8 @@ pub fn perform_federated_search( // Patch the HTTP status code to 400 as it defaults to 404 for `index_not_found`, but // here the resource not found is not part of the URL. err.code = StatusCode::BAD_REQUEST; - if let Some(query) = queries.first() { - err.message = - format!("Inside `.queries[{}]`: {}", query.query_index, err.message); + if let Some(query_index) = first_query_index { + err.message = format!("Inside `.queries[{}]`: {}", query_index, err.message); } return Err(err); } @@ -398,6 +584,23 @@ pub fn perform_federated_search( let mut used_negative_operator = false; let mut candidates = RoaringBitmap::new(); + let facets_by_index = federation.facets_by_index.remove(&index_uid).flatten(); + + // TODO: recover the max size + facets_by_index as return value of this function so as not to ask it for all queries + if let Err(mut error) = + check_facet_order(&mut facet_order, &index_uid, &facets_by_index, &index, &rtxn) + { + error.message = format!( + "Inside `.federation.facetsByIndex.{index_uid}`: {error}{}", + if let Some(query_index) = first_query_index { + format!("\n Note: index `{index_uid}` used in `.queries[{query_index}]`") + } else { + Default::default() + } + ); + return Err(error); + } + // 2.1. Compute all candidates for each query in the index let mut results_by_query = Vec::with_capacity(queries.len()); @@ -566,34 +769,118 @@ pub fn perform_federated_search( .collect(); let merged_result = merged_result?; + + let estimated_total_hits = candidates.len() as usize; + + let facets = facets_by_index + .map(|facets_by_index| { + compute_facet_distribution_stats( + &facets_by_index, + &index, + &rtxn, + candidates, + override_max_values_per_facet, + override_sort_facet_values_by, + ) + }) + .transpose() + .map_err(|mut error| { + error.message = format!( + "Inside `.federation.facetsByIndex.{index_uid}`: {}{}", + error.message, + if let Some(query_index) = first_query_index { + format!("\n Note: index `{index_uid}` used in `.queries[{query_index}]`") + } else { + Default::default() + } + ); + error + })?; + results_by_index.push(SearchResultByIndex { + index: index_uid, hits: merged_result, - candidates, + estimated_total_hits, degraded, used_negative_operator, + facets, }); } + // bonus step, make sure to return an error if an index wants a non-faceted field, even if no query actually uses that index. + for (index_uid, facets) in federation.facets_by_index { + let index = match index_scheduler.index(&index_uid) { + Ok(index) => index, + Err(err) => { + let mut err = ResponseError::from(err); + // Patch the HTTP status code to 400 as it defaults to 404 for `index_not_found`, but + // here the resource not found is not part of the URL. + err.code = StatusCode::BAD_REQUEST; + err.message = format!( + "Inside `.federation.facetsByIndex.{index_uid}`: {}\n Note: index `{index_uid}` is not used in queries", + err.message + ); + return Err(err); + } + }; + + // Important: this is the only transaction we'll use for this index during this federated search + let rtxn = index.read_txn()?; + + if let Err(mut error) = + check_facet_order(&mut facet_order, &index_uid, &facets, &index, &rtxn) + { + error.message = format!( + "Inside `.federation.facetsByIndex.{index_uid}`: {error}\n Note: index `{index_uid}` is not used in queries", + ); + return Err(error); + } + + if let Some(facets) = facets { + if let Err(mut error) = compute_facet_distribution_stats( + &facets, + &index, + &rtxn, + Default::default(), + override_max_values_per_facet, + override_sort_facet_values_by, + ) { + error.message = + format!("Inside `.federation.facetsByIndex.{index_uid}`: {}\n Note: index `{index_uid}` is not used in queries", error.message); + return Err(error); + } + } + } + // 3. merge hits and metadata across indexes // 3.1 merge metadata - let (estimated_total_hits, degraded, used_negative_operator) = { + let (estimated_total_hits, degraded, used_negative_operator, facets) = { let mut estimated_total_hits = 0; let mut degraded = false; let mut used_negative_operator = false; + let mut facets: FederatedFacets = FederatedFacets::default(); + for SearchResultByIndex { + index, hits: _, - candidates, + estimated_total_hits: estimated_total_hits_by_index, + facets: facets_by_index, degraded: degraded_by_index, used_negative_operator: used_negative_operator_by_index, - } in &results_by_index + } in &mut results_by_index { - estimated_total_hits += candidates.len() as usize; + estimated_total_hits += *estimated_total_hits_by_index; degraded |= *degraded_by_index; used_negative_operator |= *used_negative_operator_by_index; + + let facets_by_index = std::mem::take(facets_by_index); + let index = std::mem::take(index); + + facets.insert(index, facets_by_index); } - (estimated_total_hits, degraded, used_negative_operator) + (estimated_total_hits, degraded, used_negative_operator, facets) }; // 3.2 merge hits @@ -610,6 +897,18 @@ pub fn perform_federated_search( .map(|hit| hit.hit) .collect(); + let (facet_distribution, facet_stats, facets_by_index) = match federation.merge_facets { + Some(merge_facets) => { + let facets = facets.merge(merge_facets, facet_order); + + let (facet_distribution, facet_stats) = + facets.map(|ComputedFacets { distribution, stats }| (distribution, stats)).unzip(); + + (facet_distribution, facet_stats, FederatedFacets::default()) + } + None => (None, None, facets), + }; + let search_result = FederatedSearchResult { hits: merged_hits, processing_time_ms: before_search.elapsed().as_millis(), @@ -621,7 +920,39 @@ pub fn perform_federated_search( semantic_hit_count, degraded, used_negative_operator, + facet_distribution, + facet_stats, + facets_by_index, }; Ok(search_result) } + +fn check_facet_order( + facet_order: &mut Option>, + current_index: &str, + facets_by_index: &Option>, + index: &milli::Index, + rtxn: &milli::heed::RoTxn<'_>, +) -> Result<(), ResponseError> { + if let (Some(facet_order), Some(facets_by_index)) = (facet_order, facets_by_index) { + let index_facet_order = index.sort_facet_values_by(rtxn)?; + for facet in facets_by_index { + let index_facet_order = index_facet_order.get(facet); + let (previous_index, previous_facet_order) = facet_order + .entry(facet.to_owned()) + .or_insert_with(|| (current_index.to_owned(), index_facet_order)); + if previous_facet_order != &index_facet_order { + return Err(MeilisearchHttpError::InconsistentFacetOrder { + facet: facet.clone(), + previous_facet_order: *previous_facet_order, + previous_uid: previous_index.clone(), + current_uid: current_index.to_owned(), + index_facet_order, + } + .into()); + } + } + }; + Ok(()) +} From 47e3c4b5c36302b83e492c307435429f333593b8 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Thu, 12 Sep 2024 17:52:13 +0200 Subject: [PATCH 11/26] Add new tests --- meilisearch/tests/search/multi.rs | 2037 +++++++++++++++++++++++++++++ 1 file changed, 2037 insertions(+) diff --git a/meilisearch/tests/search/multi.rs b/meilisearch/tests/search/multi.rs index f92b9bfc8..f9da8877d 100644 --- a/meilisearch/tests/search/multi.rs +++ b/meilisearch/tests/search/multi.rs @@ -3855,6 +3855,214 @@ async fn federation_federated_contains_pagination() { "###); } +#[actix_rt::test] +async fn federation_federated_contains_facets() { + let server = Server::new().await; + + let index = server.index("fruits"); + + let (value, _) = index + .update_settings( + json!({"searchableAttributes": ["name"], "filterableAttributes": ["BOOST"]}), + ) + .await; + + index.wait_task(value.uid()).await; + + let documents = FRUITS_DOCUMENTS.clone(); + let (value, _) = index.add_documents(documents, None).await; + index.wait_task(value.uid()).await; + + // empty facets are actually OK + let (response, code) = server + .multi_search(json!({"federation": {}, "queries": [ + {"indexUid" : "fruits", "q": "apple red"}, + {"indexUid": "fruits", "q": "apple red", "facets": []}, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "name": "Red apple gala", + "id": "red-apple-gala", + "_federation": { + "indexUid": "fruits", + "queriesPosition": 0, + "weightedRankingScore": 0.953042328042328 + } + }, + { + "name": "Exclusive sale: Red delicious apple", + "id": "red-delicious-boosted", + "BOOST": true, + "_federation": { + "indexUid": "fruits", + "queriesPosition": 0, + "weightedRankingScore": 0.9093915343915344 + } + }, + { + "name": "Exclusive sale: green apple", + "id": "green-apple-boosted", + "BOOST": true, + "_federation": { + "indexUid": "fruits", + "queriesPosition": 0, + "weightedRankingScore": 0.4393939393939394 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 3 + } + "###); + + // fails + let (response, code) = server + .multi_search(json!({"federation": {}, "queries": [ + {"indexUid" : "fruits", "q": "apple red"}, + {"indexUid": "fruits", "q": "apple red", "facets": ["BOOSTED"]}, + ]})) + .await; + snapshot!(code, @"400 Bad Request"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "message": "Inside `.queries[1]`: Using facet options is not allowed in federated queries.\n Hint: remove `facets` from query #1 or remove `federation` from the request", + "code": "invalid_multi_search_query_facets", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_multi_search_query_facets" + } + "###); +} + +#[actix_rt::test] +async fn federation_non_faceted_for_an_index() { + let server = Server::new().await; + + let index = server.index("fruits"); + + let (value, _) = index + .update_settings( + json!({"searchableAttributes": ["name"], "filterableAttributes": ["BOOST", "id", "name"]}), + ) + .await; + + index.wait_task(value.uid()).await; + + let index = server.index("fruits-no-name"); + + let (value, _) = index + .update_settings( + json!({"searchableAttributes": ["name"], "filterableAttributes": ["BOOST", "id"]}), + ) + .await; + + index.wait_task(value.uid()).await; + + let index = server.index("fruits-no-facets"); + + let (value, _) = index.update_settings(json!({"searchableAttributes": ["name"]})).await; + + index.wait_task(value.uid()).await; + + let documents = FRUITS_DOCUMENTS.clone(); + let (value, _) = index.add_documents(documents, None).await; + index.wait_task(value.uid()).await; + + // fails + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "fruits": ["BOOST", "id", "name"], + "fruits-no-name": ["BOOST", "id", "name"], + } + }, "queries": [ + {"indexUid" : "fruits", "q": "apple red"}, + {"indexUid": "fruits-no-name", "q": "apple red"}, + ]})) + .await; + snapshot!(code, @"400 Bad Request"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "message": "Inside `.federation.facetsByIndex.fruits-no-name`: Invalid facet distribution, attribute `name` is not filterable. The available filterable attributes are `BOOST, id`.\n Note: index `fruits-no-name` used in `.queries[1]`", + "code": "invalid_search_facets", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_search_facets" + } + "###); + + // still fails + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "fruits": ["BOOST", "id", "name"], + "fruits-no-name": ["BOOST", "id", "name"], + } + }, "queries": [ + {"indexUid" : "fruits", "q": "apple red"}, + {"indexUid": "fruits", "q": "apple red"}, + ]})) + .await; + snapshot!(code, @"400 Bad Request"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "message": "Inside `.federation.facetsByIndex.fruits-no-name`: Invalid facet distribution, attribute `name` is not filterable. The available filterable attributes are `BOOST, id`.\n Note: index `fruits-no-name` is not used in queries", + "code": "invalid_search_facets", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_search_facets" + } + "###); + + // fails + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "fruits": ["BOOST", "id", "name"], + "fruits-no-name": ["BOOST", "id"], + "fruits-no-facets": ["BOOST", "id"], + } + }, "queries": [ + {"indexUid" : "fruits", "q": "apple red"}, + {"indexUid": "fruits", "q": "apple red"}, + ]})) + .await; + snapshot!(code, @"400 Bad Request"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "message": "Inside `.federation.facetsByIndex.fruits-no-facets`: Invalid facet distribution, this index does not have configured filterable attributes.\n Note: index `fruits-no-facets` is not used in queries", + "code": "invalid_search_facets", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_search_facets" + } + "###); + + // also fails + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "zorglub": ["BOOST", "id", "name"], + "fruits": ["BOOST", "id", "name"], + } + }, "queries": [ + {"indexUid" : "fruits", "q": "apple red"}, + {"indexUid": "fruits", "q": "apple red"}, + ]})) + .await; + snapshot!(code, @"400 Bad Request"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "message": "Inside `.federation.facetsByIndex.zorglub`: Index `zorglub` not found.\n Note: index `zorglub` is not used in queries", + "code": "index_not_found", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#index_not_found" + } + "###); +} + #[actix_rt::test] async fn federation_non_federated_contains_federation_option() { let server = Server::new().await; @@ -4433,3 +4641,1832 @@ async fn federation_vector_two_indexes() { } "###); } + +#[actix_rt::test] +async fn federation_facets_different_indexes_same_facet() { + let server = Server::new().await; + + let index = server.index("movies"); + + let documents = DOCUMENTS.clone(); + let (value, _) = index.add_documents(documents, None).await; + index.wait_task(value.uid()).await; + + let (value, _) = index + .update_settings(json!({ + "sortableAttributes": ["title"], + "filterableAttributes": ["title", "color"], + "rankingRules": [ + "sort", + "words", + "typo", + "proximity", + "attribute", + "exactness" + ] + })) + .await; + index.wait_task(value.uid()).await; + + let index = server.index("batman"); + + let documents = SCORE_DOCUMENTS.clone(); + let (value, _) = index.add_documents(documents, None).await; + index.wait_task(value.uid()).await; + + let (value, _) = index + .update_settings(json!({ + "sortableAttributes": ["title"], + "filterableAttributes": ["title"], + "rankingRules": [ + "sort", + "words", + "typo", + "proximity", + "attribute", + "exactness" + ] + })) + .await; + index.wait_task(value.uid()).await; + + let index = server.index("batman-2"); + + let documents = SCORE_DOCUMENTS.clone(); + let (value, _) = index.add_documents(documents, None).await; + index.wait_task(value.uid()).await; + + let (value, _) = index + .update_settings(json!({ + "sortableAttributes": ["title"], + "filterableAttributes": ["title"], + "rankingRules": [ + "sort", + "words", + "typo", + "proximity", + "attribute", + "exactness" + ] + })) + .await; + index.wait_task(value.uid()).await; + + // return titles ordered accross indexes + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "movies": ["title", "color"], + "batman": ["title"], + "batman-2": ["title"], + } + }, "queries": [ + {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "title": "Badman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Badman", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 15, + "facetsByIndex": { + "batman": { + "distribution": { + "title": { + "Badman": 1, + "Batman": 1, + "Batman Returns": 1, + "Batman the dark knight returns: Part 1": 1, + "Batman the dark knight returns: Part 2": 1 + } + }, + "stats": {} + }, + "batman-2": { + "distribution": { + "title": { + "Badman": 1, + "Batman": 1, + "Batman Returns": 1, + "Batman the dark knight returns: Part 1": 1, + "Batman the dark knight returns: Part 2": 1 + } + }, + "stats": {} + }, + "movies": { + "distribution": { + "color": { + "blue": 3, + "green": 2, + "red": 3, + "yellow": 2 + }, + "title": { + "Captain Marvel": 1, + "Escape Room": 1, + "Gläss": 1, + "How to Train Your Dragon: The Hidden World": 1, + "Shazam!": 1 + } + }, + "stats": {} + } + } + } + "###); + + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "movies": ["title"], + "batman": ["title"], + "batman-2": ["title"] + }, + "mergeFacets": {} + }, "queries": [ + {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "title": "Badman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Badman", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 15, + "facetDistribution": { + "title": { + "Badman": 2, + "Batman": 2, + "Batman Returns": 2, + "Batman the dark knight returns: Part 1": 2, + "Batman the dark knight returns: Part 2": 2, + "Captain Marvel": 1, + "Escape Room": 1, + "Gläss": 1, + "How to Train Your Dragon: The Hidden World": 1, + "Shazam!": 1 + } + }, + "facetStats": {} + } + "###); + + // mix and match query: will be sorted across indexes + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "movies": [], + "batman": ["title"], + "batman-2": ["title"] + } + }, "queries": [ + {"indexUid" : "batman", "q": "badman returns", "sort": ["title:desc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman-2", "q": "badman returns", "sort": ["title:desc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "movies", "q": "captain", "sort": ["title:desc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman", "q": "the bat", "sort": ["title:desc"], "attributesToRetrieve": ["title"] }, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies", + "queriesPosition": 2, + "weightedRankingScore": 0.9848484848484848 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman", + "queriesPosition": 3, + "weightedRankingScore": 0.9528218694885362 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 1, + "weightedRankingScore": 0.7028218694885362 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman", + "queriesPosition": 3, + "weightedRankingScore": 0.9528218694885362 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 1, + "weightedRankingScore": 0.7028218694885362 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman", + "queriesPosition": 0, + "weightedRankingScore": 0.8317901234567902 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 1, + "weightedRankingScore": 0.8317901234567902 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 0, + "weightedRankingScore": 0.23106060606060605 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 1, + "weightedRankingScore": 0.23106060606060605 + } + }, + { + "title": "Badman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 0, + "weightedRankingScore": 0.5 + } + }, + { + "title": "Badman", + "_federation": { + "indexUid": "batman-2", + "queriesPosition": 1, + "weightedRankingScore": 0.5 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 11, + "facetsByIndex": { + "batman": { + "distribution": { + "title": { + "Badman": 1, + "Batman": 1, + "Batman Returns": 1, + "Batman the dark knight returns: Part 1": 1, + "Batman the dark knight returns: Part 2": 1 + } + }, + "stats": {} + }, + "batman-2": { + "distribution": { + "title": { + "Badman": 1, + "Batman": 1, + "Batman Returns": 1, + "Batman the dark knight returns: Part 1": 1, + "Batman the dark knight returns: Part 2": 1 + } + }, + "stats": {} + }, + "movies": { + "distribution": {}, + "stats": {} + } + } + } + "###); +} + +#[actix_rt::test] +async fn federation_facets_same_indexes() { + let server = Server::new().await; + + let index = server.index("doggos"); + + let documents = NESTED_DOCUMENTS.clone(); + let (value, _) = index.add_documents(documents, None).await; + index.wait_task(value.uid()).await; + + let (value, _) = index + .update_settings(json!({ + "filterableAttributes": ["father", "mother", "doggos.age"], + "rankingRules": [ + "sort", + "words", + "typo", + "proximity", + "attribute", + "exactness" + ] + })) + .await; + index.wait_task(value.uid()).await; + + let index = server.index("doggos-2"); + + let documents = NESTED_DOCUMENTS.clone(); + let (value, _) = index.add_documents(documents, None).await; + index.wait_task(value.uid()).await; + + let (value, _) = index + .update_settings(json!({ + "filterableAttributes": ["father", "mother", "doggos.age"], + "rankingRules": [ + "sort", + "words", + "typo", + "proximity", + "attribute", + "exactness" + ] + })) + .await; + index.wait_task(value.uid()).await; + + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "doggos": ["father", "mother", "doggos.age"] + } + }, "queries": [ + {"indexUid" : "doggos", "q": "je", "attributesToRetrieve": ["id"] }, + {"indexUid" : "doggos", "q": "michel", "attributesToRetrieve": ["id"] }, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "id": 852, + "_federation": { + "indexUid": "doggos", + "queriesPosition": 0, + "weightedRankingScore": 0.9621212121212122 + } + }, + { + "id": 951, + "_federation": { + "indexUid": "doggos", + "queriesPosition": 0, + "weightedRankingScore": 0.9621212121212122 + } + }, + { + "id": 750, + "_federation": { + "indexUid": "doggos", + "queriesPosition": 1, + "weightedRankingScore": 0.9621212121212122 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 3, + "facetsByIndex": { + "doggos": { + "distribution": { + "doggos.age": { + "2": 1, + "4": 1, + "5": 1, + "6": 1 + }, + "father": { + "jean": 1, + "jean-baptiste": 1, + "romain": 1 + }, + "mother": { + "michelle": 2, + "sophie": 1 + } + }, + "stats": { + "doggos.age": { + "min": 2.0, + "max": 6.0 + } + } + } + } + } + "###); + + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "doggos": ["father", "mother", "doggos.age"], + "doggos-2": ["father", "mother", "doggos.age"] + } + }, "queries": [ + {"indexUid" : "doggos", "q": "je", "attributesToRetrieve": ["id"] }, + {"indexUid" : "doggos-2", "q": "michel", "attributesToRetrieve": ["id"] }, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "id": 852, + "_federation": { + "indexUid": "doggos", + "queriesPosition": 0, + "weightedRankingScore": 0.9621212121212122 + } + }, + { + "id": 951, + "_federation": { + "indexUid": "doggos", + "queriesPosition": 0, + "weightedRankingScore": 0.9621212121212122 + } + }, + { + "id": 852, + "_federation": { + "indexUid": "doggos-2", + "queriesPosition": 1, + "weightedRankingScore": 0.9621212121212122 + } + }, + { + "id": 750, + "_federation": { + "indexUid": "doggos-2", + "queriesPosition": 1, + "weightedRankingScore": 0.9621212121212122 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 4, + "facetsByIndex": { + "doggos": { + "distribution": { + "doggos.age": { + "2": 1, + "4": 1, + "5": 1, + "6": 1 + }, + "father": { + "jean": 1, + "jean-baptiste": 1 + }, + "mother": { + "michelle": 1, + "sophie": 1 + } + }, + "stats": { + "doggos.age": { + "min": 2.0, + "max": 6.0 + } + } + }, + "doggos-2": { + "distribution": { + "doggos.age": { + "2": 1, + "4": 1 + }, + "father": { + "jean": 1, + "romain": 1 + }, + "mother": { + "michelle": 2 + } + }, + "stats": { + "doggos.age": { + "min": 2.0, + "max": 4.0 + } + } + } + } + } + "###); + + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "doggos": ["father", "mother", "doggos.age"], + "doggos-2": ["father", "mother", "doggos.age"] + }, + "mergeFacets": {}, + }, "queries": [ + {"indexUid" : "doggos", "q": "je", "attributesToRetrieve": ["id"] }, + {"indexUid" : "doggos-2", "q": "michel", "attributesToRetrieve": ["id"] }, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "id": 852, + "_federation": { + "indexUid": "doggos", + "queriesPosition": 0, + "weightedRankingScore": 0.9621212121212122 + } + }, + { + "id": 951, + "_federation": { + "indexUid": "doggos", + "queriesPosition": 0, + "weightedRankingScore": 0.9621212121212122 + } + }, + { + "id": 852, + "_federation": { + "indexUid": "doggos-2", + "queriesPosition": 1, + "weightedRankingScore": 0.9621212121212122 + } + }, + { + "id": 750, + "_federation": { + "indexUid": "doggos-2", + "queriesPosition": 1, + "weightedRankingScore": 0.9621212121212122 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 4, + "facetDistribution": { + "doggos.age": { + "2": 2, + "4": 2, + "5": 1, + "6": 1 + }, + "father": { + "jean": 2, + "jean-baptiste": 1, + "romain": 1 + }, + "mother": { + "michelle": 3, + "sophie": 1 + } + }, + "facetStats": { + "doggos.age": { + "min": 2.0, + "max": 6.0 + } + } + } + "###); +} + +#[actix_rt::test] +async fn federation_inconsistent_merge_order() { + let server = Server::new().await; + + let index = server.index("movies"); + + let documents = DOCUMENTS.clone(); + let (value, _) = index.add_documents(documents, None).await; + index.wait_task(value.uid()).await; + + let (value, _) = index + .update_settings(json!({ + "sortableAttributes": ["title"], + "filterableAttributes": ["title", "color"], + "rankingRules": [ + "sort", + "words", + "typo", + "proximity", + "attribute", + "exactness" + ] + })) + .await; + index.wait_task(value.uid()).await; + + let index = server.index("movies-2"); + + let documents = DOCUMENTS.clone(); + let (value, _) = index.add_documents(documents, None).await; + index.wait_task(value.uid()).await; + + let (value, _) = index + .update_settings(json!({ + "sortableAttributes": ["title"], + "filterableAttributes": ["title", "color"], + "rankingRules": [ + "sort", + "words", + "typo", + "proximity", + "attribute", + "exactness" + ], + "faceting": { + "sortFacetValuesBy": { "color": "count" } + } + })) + .await; + index.wait_task(value.uid()).await; + + let index = server.index("batman"); + + let documents = SCORE_DOCUMENTS.clone(); + let (value, _) = index.add_documents(documents, None).await; + index.wait_task(value.uid()).await; + + let (value, _) = index + .update_settings(json!({ + "sortableAttributes": ["title"], + "filterableAttributes": ["title"], + "rankingRules": [ + "sort", + "words", + "typo", + "proximity", + "attribute", + "exactness" + ] + })) + .await; + index.wait_task(value.uid()).await; + + // without merging, it works + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "movies": ["title", "color"], + "batman": ["title"], + "movies-2": ["title", "color"], + } + }, "queries": [ + {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "movies-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "title": "Badman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 15, + "facetsByIndex": { + "batman": { + "distribution": { + "title": { + "Badman": 1, + "Batman": 1, + "Batman Returns": 1, + "Batman the dark knight returns: Part 1": 1, + "Batman the dark knight returns: Part 2": 1 + } + }, + "stats": {} + }, + "movies": { + "distribution": { + "color": { + "blue": 3, + "green": 2, + "red": 3, + "yellow": 2 + }, + "title": { + "Captain Marvel": 1, + "Escape Room": 1, + "Gläss": 1, + "How to Train Your Dragon: The Hidden World": 1, + "Shazam!": 1 + } + }, + "stats": {} + }, + "movies-2": { + "distribution": { + "color": { + "red": 3, + "blue": 3, + "yellow": 2, + "green": 2 + }, + "title": { + "Captain Marvel": 1, + "Escape Room": 1, + "Gläss": 1, + "How to Train Your Dragon: The Hidden World": 1, + "Shazam!": 1 + } + }, + "stats": {} + } + } + } + "###); + + // fails with merging + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "movies": ["title", "color"], + "batman": ["title"], + "movies-2": ["title", "color"], + }, + "mergeFacets": {} + }, "queries": [ + {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "movies-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + ]})) + .await; + snapshot!(code, @"400 Bad Request"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "message": "Inside `.federation.facetsByIndex.movies-2`: Inconsistent order for values in facet `color`: index `movies` orders alphabetically, but index `movies-2` orders by count.\n Hint: Remove `federation.mergeFacets` or set `federation.mergeFacets.sortFacetValuesBy` to the desired order.\n Note: index `movies-2` used in `.queries[2]`", + "code": "invalid_multi_search_facet_order", + "type": "invalid_request", + "link": "https://docs.meilisearch.com/errors#invalid_multi_search_facet_order" + } + "###); + + // works again with merging and forcing an order + let (response, code) = server +.multi_search(json!({"federation": { + "facetsByIndex": { + "movies": ["title", "color"], + "batman": ["title"], + "movies-2": ["title", "color"], + }, + "mergeFacets": { + "sortFacetValuesBy": "count" + } +}, "queries": [ + {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "movies-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, +]})) +.await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "title": "Badman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 15, + "facetDistribution": { + "color": { + "red": 6, + "blue": 6, + "yellow": 4, + "green": 4 + }, + "title": { + "Shazam!": 2, + "How to Train Your Dragon: The Hidden World": 2, + "Gläss": 2, + "Escape Room": 2, + "Captain Marvel": 2, + "Batman the dark knight returns: Part 2": 1, + "Batman the dark knight returns: Part 1": 1, + "Batman Returns": 1, + "Batman": 1, + "Badman": 1 + } + }, + "facetStats": {} + } + "###); + + // works also with the other order + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "movies": ["title", "color"], + "batman": ["title"], + "movies-2": ["title", "color"], + }, + "mergeFacets": { + "sortFacetValuesBy": "alpha" + } + }, "queries": [ + {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "movies-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "title": "Badman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 15, + "facetDistribution": { + "color": { + "blue": 6, + "green": 4, + "red": 6, + "yellow": 4 + }, + "title": { + "Badman": 1, + "Batman": 1, + "Batman Returns": 1, + "Batman the dark knight returns: Part 1": 1, + "Batman the dark knight returns: Part 2": 1, + "Captain Marvel": 2, + "Escape Room": 2, + "Gläss": 2, + "How to Train Your Dragon: The Hidden World": 2, + "Shazam!": 2 + } + }, + "facetStats": {} + } + "###); + + // can limit the number of values + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "movies": ["title", "color"], + "batman": ["title"], + "movies-2": ["title", "color"], + }, + "mergeFacets": { + "sortFacetValuesBy": "count", + "maxValuesPerFacet": 3, + } + }, "queries": [ + {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "movies-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "title": "Badman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 15, + "facetDistribution": { + "color": { + "red": 6, + "blue": 6, + "yellow": 4 + }, + "title": { + "Shazam!": 2, + "How to Train Your Dragon: The Hidden World": 2, + "Gläss": 2 + } + }, + "facetStats": {} + } + "###); + + // can limit the number of values by alpha + let (response, code) = server + .multi_search(json!({"federation": { + "facetsByIndex": { + "movies": ["title", "color"], + "batman": ["title"], + "movies-2": ["title", "color"], + }, + "mergeFacets": { + "sortFacetValuesBy": "alpha", + "maxValuesPerFacet": 3, + } + }, "queries": [ + {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + {"indexUid" : "movies-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, + ]})) + .await; + snapshot!(code, @"200 OK"); + insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" + { + "hits": [ + { + "title": "Badman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman Returns", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 1", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Batman the dark knight returns: Part 2", + "_federation": { + "indexUid": "batman", + "queriesPosition": 1, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Captain Marvel", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Escape Room", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Gläss", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "How to Train Your Dragon: The Hidden World", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies", + "queriesPosition": 0, + "weightedRankingScore": 1.0 + } + }, + { + "title": "Shazam!", + "_federation": { + "indexUid": "movies-2", + "queriesPosition": 2, + "weightedRankingScore": 1.0 + } + } + ], + "processingTimeMs": "[time]", + "limit": 20, + "offset": 0, + "estimatedTotalHits": 15, + "facetDistribution": { + "color": { + "blue": 6, + "green": 4, + "red": 6 + }, + "title": { + "Badman": 1, + "Batman": 1, + "Batman Returns": 1 + } + }, + "facetStats": {} + } + "###); +} From 91dfab317f2c53f1556fcd3eacb8cb1980c50fce Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Mon, 16 Sep 2024 15:17:46 +0200 Subject: [PATCH 12/26] New error --- meilisearch-types/src/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/meilisearch-types/src/error.rs b/meilisearch-types/src/error.rs index bf89fe614..d443e5709 100644 --- a/meilisearch-types/src/error.rs +++ b/meilisearch-types/src/error.rs @@ -238,6 +238,7 @@ InvalidIndexLimit , InvalidRequest , BAD_REQUEST ; InvalidIndexOffset , InvalidRequest , BAD_REQUEST ; InvalidIndexPrimaryKey , InvalidRequest , BAD_REQUEST ; InvalidIndexUid , InvalidRequest , BAD_REQUEST ; +InvalidMultiSearchFacets , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchFacetsByIndex , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchFacetOrder , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchFederated , InvalidRequest , BAD_REQUEST ; From 38c4be1c8e6de30b237179fc8eca15f9cf4eb08c Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Mon, 16 Sep 2024 15:18:09 +0200 Subject: [PATCH 13/26] compute_facets accepts Route argument to fixup error code --- meilisearch/src/search/mod.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/meilisearch/src/search/mod.rs b/meilisearch/src/search/mod.rs index 99245bdc1..13cfb9334 100644 --- a/meilisearch/src/search/mod.rs +++ b/meilisearch/src/search/mod.rs @@ -991,7 +991,15 @@ pub fn perform_search( let (facet_distribution, facet_stats) = facets .map(move |facets| { - compute_facet_distribution_stats(&facets, index, &rtxn, candidates, None, None) + compute_facet_distribution_stats( + &facets, + index, + &rtxn, + candidates, + None, + None, + Route::Search, + ) }) .transpose()? .map(|ComputedFacets { distribution, stats }| (distribution, stats)) @@ -1017,6 +1025,11 @@ pub struct ComputedFacets { pub stats: BTreeMap, } +enum Route { + Search, + MultiSearch, +} + fn compute_facet_distribution_stats>( facets: &[S], index: &Index, @@ -1024,6 +1037,7 @@ fn compute_facet_distribution_stats>( candidates: roaring::RoaringBitmap, override_max_values_per_facet: Option, override_sort_facet_values_by: Option, + route: Route, ) -> Result { let mut facet_distribution = index.facets_distribution(rtxn); @@ -1054,7 +1068,16 @@ fn compute_facet_distribution_stats>( let distribution = facet_distribution .candidates(candidates) .default_order_by(sort_facet_values_by("*")) - .execute()?; + .execute() + .map_err(|error| match (error, route) { + ( + error @ milli::Error::UserError(milli::UserError::InvalidFacetsDistribution { + .. + }), + Route::MultiSearch, + ) => ResponseError::from_msg(error.to_string(), Code::InvalidMultiSearchFacets), + (error, _) => error.into(), + })?; let stats = facet_distribution.compute_stats()?; let stats = stats.into_iter().map(|(k, (min, max))| (k, FacetStats { min, max })).collect(); Ok(ComputedFacets { distribution, stats }) From 95da428dc8c1ed55dcb8e8b866caf974e0569be4 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Mon, 16 Sep 2024 15:18:23 +0200 Subject: [PATCH 14/26] Use route in federated --- meilisearch/src/search/federated.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/meilisearch/src/search/federated.rs b/meilisearch/src/search/federated.rs index 9d16ca59d..6470002ab 100644 --- a/meilisearch/src/search/federated.rs +++ b/meilisearch/src/search/federated.rs @@ -781,6 +781,7 @@ pub fn perform_federated_search( candidates, override_max_values_per_facet, override_sort_facet_values_by, + super::Route::MultiSearch, ) }) .transpose() @@ -844,6 +845,7 @@ pub fn perform_federated_search( Default::default(), override_max_values_per_facet, override_sort_facet_values_by, + super::Route::MultiSearch, ) { error.message = format!("Inside `.federation.facetsByIndex.{index_uid}`: {}\n Note: index `{index_uid}` is not used in queries", error.message); From 6732dd95d77f605bf946139844e5379d57fed320 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Mon, 16 Sep 2024 15:18:32 +0200 Subject: [PATCH 15/26] Update tests --- meilisearch/tests/search/multi.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/meilisearch/tests/search/multi.rs b/meilisearch/tests/search/multi.rs index f9da8877d..0eeca4ce9 100644 --- a/meilisearch/tests/search/multi.rs +++ b/meilisearch/tests/search/multi.rs @@ -3989,9 +3989,9 @@ async fn federation_non_faceted_for_an_index() { insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { "message": "Inside `.federation.facetsByIndex.fruits-no-name`: Invalid facet distribution, attribute `name` is not filterable. The available filterable attributes are `BOOST, id`.\n Note: index `fruits-no-name` used in `.queries[1]`", - "code": "invalid_search_facets", + "code": "invalid_multi_search_facets", "type": "invalid_request", - "link": "https://docs.meilisearch.com/errors#invalid_search_facets" + "link": "https://docs.meilisearch.com/errors#invalid_multi_search_facets" } "###); @@ -4011,9 +4011,9 @@ async fn federation_non_faceted_for_an_index() { insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { "message": "Inside `.federation.facetsByIndex.fruits-no-name`: Invalid facet distribution, attribute `name` is not filterable. The available filterable attributes are `BOOST, id`.\n Note: index `fruits-no-name` is not used in queries", - "code": "invalid_search_facets", + "code": "invalid_multi_search_facets", "type": "invalid_request", - "link": "https://docs.meilisearch.com/errors#invalid_search_facets" + "link": "https://docs.meilisearch.com/errors#invalid_multi_search_facets" } "###); @@ -4034,9 +4034,9 @@ async fn federation_non_faceted_for_an_index() { insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { "message": "Inside `.federation.facetsByIndex.fruits-no-facets`: Invalid facet distribution, this index does not have configured filterable attributes.\n Note: index `fruits-no-facets` is not used in queries", - "code": "invalid_search_facets", + "code": "invalid_multi_search_facets", "type": "invalid_request", - "link": "https://docs.meilisearch.com/errors#invalid_search_facets" + "link": "https://docs.meilisearch.com/errors#invalid_multi_search_facets" } "###); From dc8a662209395378dc9a6c17e3f1def3f00a218c Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 17 Sep 2024 10:08:21 +0200 Subject: [PATCH 16/26] federated queries: adjust error message --- meilisearch/src/error.rs | 8 ++++---- meilisearch/src/search/federated.rs | 10 ++++++++-- meilisearch/src/search/mod.rs | 5 ++--- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/meilisearch/src/error.rs b/meilisearch/src/error.rs index fa315837f..9d5eff016 100644 --- a/meilisearch/src/error.rs +++ b/meilisearch/src/error.rs @@ -32,9 +32,9 @@ pub enum MeilisearchHttpError { FederationOptionsInNonFederatedRequest(usize), #[error("Inside `.queries[{0}]`: Using pagination options is not allowed in federated queries.\n - Hint: remove `{1}` from query #{0} or remove `federation` from the request\n - Hint: pass `federation.limit` and `federation.offset` for pagination in federated search")] PaginationInFederatedQuery(usize, &'static str), - #[error("Inside `.queries[{0}]`: Using facet options is not allowed in federated queries.\n Hint: remove `facets` from query #{0} or remove `federation` from the request")] - FacetsInFederatedQuery(usize), - #[error("Inconsistent order for values in facet `{facet}`: index `{previous_uid}` orders {previous_facet_order}, but index `{current_uid}` orders {index_facet_order}.\n Hint: Remove `federation.mergeFacets` or set `federation.mergeFacets.sortFacetValuesBy` to the desired order.")] + #[error("Inside `.queries[{0}]`: Using facet options is not allowed in federated queries.\n - Hint: remove `facets` from query #{0} or remove `federation` from the request\n - Hint: pass `federation.facetsByIndex.{1}: {2:?}` for facets in federated search")] + FacetsInFederatedQuery(usize, String, Vec), + #[error("Inconsistent order for values in facet `{facet}`: index `{previous_uid}` orders {previous_facet_order}, but index `{current_uid}` orders {index_facet_order}.\n - Hint: Remove `federation.mergeFacets` or set `federation.mergeFacets.sortFacetValuesBy` to the desired order.")] InconsistentFacetOrder { facet: String, previous_facet_order: OrderBy, @@ -107,7 +107,7 @@ impl ErrorCode for MeilisearchHttpError { MeilisearchHttpError::PaginationInFederatedQuery(_, _) => { Code::InvalidMultiSearchQueryPagination } - MeilisearchHttpError::FacetsInFederatedQuery(_) => Code::InvalidMultiSearchQueryFacets, + MeilisearchHttpError::FacetsInFederatedQuery(..) => Code::InvalidMultiSearchQueryFacets, MeilisearchHttpError::InconsistentFacetOrder { .. } => { Code::InvalidMultiSearchFacetOrder } diff --git a/meilisearch/src/search/federated.rs b/meilisearch/src/search/federated.rs index 6470002ab..46643556d 100644 --- a/meilisearch/src/search/federated.rs +++ b/meilisearch/src/search/federated.rs @@ -513,8 +513,14 @@ pub fn perform_federated_search( .into()); } - if federated_query.has_facets() { - return Err(MeilisearchHttpError::FacetsInFederatedQuery(query_index).into()); + if let Some(facets) = federated_query.has_facets() { + let facets = facets.to_owned(); + return Err(MeilisearchHttpError::FacetsInFederatedQuery( + query_index, + federated_query.index_uid.into_inner(), + facets, + ) + .into()); } let (index_uid, query, federation_options) = federated_query.into_index_query_federation(); diff --git a/meilisearch/src/search/mod.rs b/meilisearch/src/search/mod.rs index 13cfb9334..4d5d8d890 100644 --- a/meilisearch/src/search/mod.rs +++ b/meilisearch/src/search/mod.rs @@ -455,9 +455,8 @@ impl SearchQueryWithIndex { } } - pub fn has_facets(&self) -> bool { - let Some(facets) = &self.facets else { return false }; - !facets.is_empty() + pub fn has_facets(&self) -> Option<&[String]> { + self.facets.as_deref().filter(|v| !v.is_empty()) } pub fn into_index_query_federation(self) -> (IndexUid, SearchQuery, Option) { From d9e0df74eaa7e0f00298520a3181183b14b8bec5 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 17 Sep 2024 10:09:01 +0200 Subject: [PATCH 17/26] update test --- meilisearch/tests/search/multi.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/meilisearch/tests/search/multi.rs b/meilisearch/tests/search/multi.rs index 0eeca4ce9..662d10a4c 100644 --- a/meilisearch/tests/search/multi.rs +++ b/meilisearch/tests/search/multi.rs @@ -3931,7 +3931,7 @@ async fn federation_federated_contains_facets() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.queries[1]`: Using facet options is not allowed in federated queries.\n Hint: remove `facets` from query #1 or remove `federation` from the request", + "message": "Inside `.queries[1]`: Using facet options is not allowed in federated queries.\n - Hint: remove `facets` from query #1 or remove `federation` from the request\n - Hint: pass `federation.facetsByIndex.fruits: [\"BOOSTED\"]` for facets in federated search", "code": "invalid_multi_search_query_facets", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_query_facets" @@ -5797,7 +5797,7 @@ async fn federation_inconsistent_merge_order() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.federation.facetsByIndex.movies-2`: Inconsistent order for values in facet `color`: index `movies` orders alphabetically, but index `movies-2` orders by count.\n Hint: Remove `federation.mergeFacets` or set `federation.mergeFacets.sortFacetValuesBy` to the desired order.\n Note: index `movies-2` used in `.queries[2]`", + "message": "Inside `.federation.facetsByIndex.movies-2`: Inconsistent order for values in facet `color`: index `movies` orders alphabetically, but index `movies-2` orders by count.\n - Hint: Remove `federation.mergeFacets` or set `federation.mergeFacets.sortFacetValuesBy` to the desired order.\n Note: index `movies-2` used in `.queries[2]`", "code": "invalid_multi_search_facet_order", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_facet_order" From 98b77aec668cc43bb15d4e5c373d3426d64c5fc3 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 17 Sep 2024 17:22:03 +0200 Subject: [PATCH 18/26] Remove runtime sortFacetValuesBy --- meilisearch-types/src/error.rs | 1 - meilisearch/src/search/federated.rs | 81 ++++++++--------------------- 2 files changed, 21 insertions(+), 61 deletions(-) diff --git a/meilisearch-types/src/error.rs b/meilisearch-types/src/error.rs index d443e5709..535bf2dd6 100644 --- a/meilisearch-types/src/error.rs +++ b/meilisearch-types/src/error.rs @@ -248,7 +248,6 @@ InvalidMultiSearchMergeFacets , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchQueryFacets , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchQueryPagination , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchQueryRankingRules , InvalidRequest , BAD_REQUEST ; -InvalidMultiSearchSortFacetValuesBy , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchWeight , InvalidRequest , BAD_REQUEST ; InvalidSearchAttributesToSearchOn , InvalidRequest , BAD_REQUEST ; InvalidSearchAttributesToCrop , InvalidRequest , BAD_REQUEST ; diff --git a/meilisearch/src/search/federated.rs b/meilisearch/src/search/federated.rs index 46643556d..804d56689 100644 --- a/meilisearch/src/search/federated.rs +++ b/meilisearch/src/search/federated.rs @@ -13,8 +13,8 @@ use indexmap::IndexMap; use meilisearch_types::deserr::DeserrJsonError; use meilisearch_types::error::deserr_codes::{ InvalidMultiSearchFacetsByIndex, InvalidMultiSearchMaxValuesPerFacet, - InvalidMultiSearchMergeFacets, InvalidMultiSearchSortFacetValuesBy, InvalidMultiSearchWeight, - InvalidSearchLimit, InvalidSearchOffset, + InvalidMultiSearchMergeFacets, InvalidMultiSearchWeight, InvalidSearchLimit, + InvalidSearchOffset, }; use meilisearch_types::error::ResponseError; use meilisearch_types::index_uid::IndexUid; @@ -86,44 +86,10 @@ pub struct Federation { #[derive(Copy, Clone, Debug, deserr::Deserr, Default)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] pub struct MergeFacets { - #[deserr(default, error = DeserrJsonError)] - pub sort_facet_values_by: SortFacetValuesBy, #[deserr(default, error = DeserrJsonError)] pub max_values_per_facet: Option, } -impl MergeFacets { - pub fn to_components(this: Option) -> (Option, Option) { - match this { - Some(MergeFacets { sort_facet_values_by, max_values_per_facet }) => { - (sort_facet_values_by.into(), max_values_per_facet) - } - None => (None, None), - } - } -} - -#[derive(Debug, deserr::Deserr, Default, Clone, Copy)] -#[deserr(rename_all = camelCase, deny_unknown_fields)] -pub enum SortFacetValuesBy { - #[default] - IndexSettings, - /// By lexicographic order... - Alpha, - /// Or by number of docids in common? - Count, -} - -impl From for Option { - fn from(value: SortFacetValuesBy) -> Self { - match value { - SortFacetValuesBy::Alpha => Some(OrderBy::Lexicographic), - SortFacetValuesBy::Count => Some(OrderBy::Count), - SortFacetValuesBy::IndexSettings => None, - } - } -} - #[derive(Debug, deserr::Deserr, Default)] #[deserr(rename_all = camelCase, deny_unknown_fields)] pub enum GroupFacetsBy { @@ -413,8 +379,8 @@ impl FederatedFacets { pub fn merge( self, - MergeFacets { sort_facet_values_by, max_values_per_facet }: MergeFacets, - facet_order: Option>, + MergeFacets { max_values_per_facet }: MergeFacets, + facet_order: BTreeMap, ) -> Option { if self.is_empty() { return None; @@ -461,12 +427,7 @@ impl FederatedFacets { // fixup order for (facet, values) in &mut distribution { - let order_by = Option::::from(sort_facet_values_by) - .or_else(|| match &facet_order { - Some(facet_order) => facet_order.get(facet).map(|(_, order)| *order), - None => None, - }) - .unwrap_or_default(); + let order_by = facet_order.get(facet).map(|(_, order)| *order).unwrap_or_default(); match order_by { OrderBy::Lexicographic => { @@ -535,8 +496,8 @@ pub fn perform_federated_search( // 2. perform queries, merge and make hits index by index let required_hit_count = federation.limit + federation.offset; - let (override_sort_facet_values_by, override_max_values_per_facet) = - MergeFacets::to_components(federation.merge_facets); + let override_max_values_per_facet = + federation.merge_facets.and_then(|merge_facets| merge_facets.max_values_per_facet); // In step (2), semantic_hit_count will be set to Some(0) if any search kind uses semantic // Then in step (3), we'll update its value if there is any semantic search @@ -548,9 +509,7 @@ pub fn perform_federated_search( // to detect if the order is inconsistent for a facet. let mut facet_order: Option> = match federation.merge_facets { - Some(MergeFacets { sort_facet_values_by: SortFacetValuesBy::IndexSettings, .. }) => { - Some(Default::default()) - } + Some(MergeFacets { .. }) => Some(Default::default()), _ => None, }; @@ -786,7 +745,7 @@ pub fn perform_federated_search( &rtxn, candidates, override_max_values_per_facet, - override_sort_facet_values_by, + None, super::Route::MultiSearch, ) }) @@ -850,7 +809,7 @@ pub fn perform_federated_search( &rtxn, Default::default(), override_max_values_per_facet, - override_sort_facet_values_by, + None, super::Route::MultiSearch, ) { error.message = @@ -905,17 +864,19 @@ pub fn perform_federated_search( .map(|hit| hit.hit) .collect(); - let (facet_distribution, facet_stats, facets_by_index) = match federation.merge_facets { - Some(merge_facets) => { - let facets = facets.merge(merge_facets, facet_order); + let (facet_distribution, facet_stats, facets_by_index) = + match federation.merge_facets.zip(facet_order) { + Some((merge_facets, facet_order)) => { + let facets = facets.merge(merge_facets, facet_order); - let (facet_distribution, facet_stats) = - facets.map(|ComputedFacets { distribution, stats }| (distribution, stats)).unzip(); + let (facet_distribution, facet_stats) = facets + .map(|ComputedFacets { distribution, stats }| (distribution, stats)) + .unzip(); - (facet_distribution, facet_stats, FederatedFacets::default()) - } - None => (None, None, facets), - }; + (facet_distribution, facet_stats, FederatedFacets::default()) + } + None => (None, None, facets), + }; let search_result = FederatedSearchResult { hits: merged_hits, From c42746c4cd79349122d29095d5df0d378966aa59 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 17 Sep 2024 17:22:14 +0200 Subject: [PATCH 19/26] Update tests --- meilisearch/tests/search/multi.rs | 512 +----------------------------- 1 file changed, 4 insertions(+), 508 deletions(-) diff --git a/meilisearch/tests/search/multi.rs b/meilisearch/tests/search/multi.rs index 662d10a4c..7cf4bd415 100644 --- a/meilisearch/tests/search/multi.rs +++ b/meilisearch/tests/search/multi.rs @@ -5804,356 +5804,15 @@ async fn federation_inconsistent_merge_order() { } "###); - // works again with merging and forcing an order - let (response, code) = server -.multi_search(json!({"federation": { - "facetsByIndex": { - "movies": ["title", "color"], - "batman": ["title"], - "movies-2": ["title", "color"], - }, - "mergeFacets": { - "sortFacetValuesBy": "count" - } -}, "queries": [ - {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, - {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, - {"indexUid" : "movies-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, -]})) -.await; - snapshot!(code, @"200 OK"); - insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" - { - "hits": [ - { - "title": "Badman", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman Returns", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman the dark knight returns: Part 1", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman the dark knight returns: Part 2", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Captain Marvel", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Captain Marvel", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Escape Room", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Escape Room", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Gläss", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Gläss", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "How to Train Your Dragon: The Hidden World", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "How to Train Your Dragon: The Hidden World", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Shazam!", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Shazam!", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - } - ], - "processingTimeMs": "[time]", - "limit": 20, - "offset": 0, - "estimatedTotalHits": 15, - "facetDistribution": { - "color": { - "red": 6, - "blue": 6, - "yellow": 4, - "green": 4 - }, - "title": { - "Shazam!": 2, - "How to Train Your Dragon: The Hidden World": 2, - "Gläss": 2, - "Escape Room": 2, - "Captain Marvel": 2, - "Batman the dark knight returns: Part 2": 1, - "Batman the dark knight returns: Part 1": 1, - "Batman Returns": 1, - "Batman": 1, - "Badman": 1 - } - }, - "facetStats": {} - } - "###); - - // works also with the other order - let (response, code) = server - .multi_search(json!({"federation": { - "facetsByIndex": { - "movies": ["title", "color"], - "batman": ["title"], - "movies-2": ["title", "color"], - }, - "mergeFacets": { - "sortFacetValuesBy": "alpha" - } - }, "queries": [ - {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, - {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, - {"indexUid" : "movies-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, - ]})) - .await; - snapshot!(code, @"200 OK"); - insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" - { - "hits": [ - { - "title": "Badman", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman Returns", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman the dark knight returns: Part 1", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman the dark knight returns: Part 2", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Captain Marvel", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Captain Marvel", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Escape Room", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Escape Room", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Gläss", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Gläss", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "How to Train Your Dragon: The Hidden World", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "How to Train Your Dragon: The Hidden World", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Shazam!", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Shazam!", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - } - ], - "processingTimeMs": "[time]", - "limit": 20, - "offset": 0, - "estimatedTotalHits": 15, - "facetDistribution": { - "color": { - "blue": 6, - "green": 4, - "red": 6, - "yellow": 4 - }, - "title": { - "Badman": 1, - "Batman": 1, - "Batman Returns": 1, - "Batman the dark knight returns: Part 1": 1, - "Batman the dark knight returns: Part 2": 1, - "Captain Marvel": 2, - "Escape Room": 2, - "Gläss": 2, - "How to Train Your Dragon: The Hidden World": 2, - "Shazam!": 2 - } - }, - "facetStats": {} - } - "###); - // can limit the number of values let (response, code) = server .multi_search(json!({"federation": { "facetsByIndex": { "movies": ["title", "color"], "batman": ["title"], - "movies-2": ["title", "color"], + "movies-2": ["title"], }, "mergeFacets": { - "sortFacetValuesBy": "count", "maxValuesPerFacet": 3, } }, "queries": [ @@ -6293,172 +5952,9 @@ async fn federation_inconsistent_merge_order() { "estimatedTotalHits": 15, "facetDistribution": { "color": { - "red": 6, - "blue": 6, - "yellow": 4 - }, - "title": { - "Shazam!": 2, - "How to Train Your Dragon: The Hidden World": 2, - "Gläss": 2 - } - }, - "facetStats": {} - } - "###); - - // can limit the number of values by alpha - let (response, code) = server - .multi_search(json!({"federation": { - "facetsByIndex": { - "movies": ["title", "color"], - "batman": ["title"], - "movies-2": ["title", "color"], - }, - "mergeFacets": { - "sortFacetValuesBy": "alpha", - "maxValuesPerFacet": 3, - } - }, "queries": [ - {"indexUid" : "movies", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, - {"indexUid" : "batman", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, - {"indexUid" : "movies-2", "q": "", "sort": ["title:asc"], "attributesToRetrieve": ["title"] }, - ]})) - .await; - snapshot!(code, @"200 OK"); - insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" - { - "hits": [ - { - "title": "Badman", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman Returns", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman the dark knight returns: Part 1", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Batman the dark knight returns: Part 2", - "_federation": { - "indexUid": "batman", - "queriesPosition": 1, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Captain Marvel", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Captain Marvel", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Escape Room", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Escape Room", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Gläss", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Gläss", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "How to Train Your Dragon: The Hidden World", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "How to Train Your Dragon: The Hidden World", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Shazam!", - "_federation": { - "indexUid": "movies", - "queriesPosition": 0, - "weightedRankingScore": 1.0 - } - }, - { - "title": "Shazam!", - "_federation": { - "indexUid": "movies-2", - "queriesPosition": 2, - "weightedRankingScore": 1.0 - } - } - ], - "processingTimeMs": "[time]", - "limit": 20, - "offset": 0, - "estimatedTotalHits": 15, - "facetDistribution": { - "color": { - "blue": 6, - "green": 4, - "red": 6 + "blue": 3, + "green": 2, + "red": 3 }, "title": { "Badman": 1, From af8edab21df07dcd01b055174105febe36a0f92e Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 17 Sep 2024 17:39:51 +0200 Subject: [PATCH 20/26] Remove mention of sort order and recommend changing index settings on inconsistent order error --- meilisearch/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meilisearch/src/error.rs b/meilisearch/src/error.rs index 9d5eff016..b3a94e60d 100644 --- a/meilisearch/src/error.rs +++ b/meilisearch/src/error.rs @@ -34,7 +34,7 @@ pub enum MeilisearchHttpError { PaginationInFederatedQuery(usize, &'static str), #[error("Inside `.queries[{0}]`: Using facet options is not allowed in federated queries.\n - Hint: remove `facets` from query #{0} or remove `federation` from the request\n - Hint: pass `federation.facetsByIndex.{1}: {2:?}` for facets in federated search")] FacetsInFederatedQuery(usize, String, Vec), - #[error("Inconsistent order for values in facet `{facet}`: index `{previous_uid}` orders {previous_facet_order}, but index `{current_uid}` orders {index_facet_order}.\n - Hint: Remove `federation.mergeFacets` or set `federation.mergeFacets.sortFacetValuesBy` to the desired order.")] + #[error("Inconsistent order for values in facet `{facet}`: index `{previous_uid}` orders {previous_facet_order}, but index `{current_uid}` orders {index_facet_order}.\n - Hint: Remove `federation.mergeFacets` or change `faceting.sortFacetValuesBy` to be consistent in settings.")] InconsistentFacetOrder { facet: String, previous_facet_order: OrderBy, From df648ce7a63fa59c0d72d1d010a477dda585a301 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 17 Sep 2024 17:40:14 +0200 Subject: [PATCH 21/26] Update tests --- meilisearch/tests/search/multi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meilisearch/tests/search/multi.rs b/meilisearch/tests/search/multi.rs index 7cf4bd415..1a2ca4c84 100644 --- a/meilisearch/tests/search/multi.rs +++ b/meilisearch/tests/search/multi.rs @@ -5797,7 +5797,7 @@ async fn federation_inconsistent_merge_order() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.federation.facetsByIndex.movies-2`: Inconsistent order for values in facet `color`: index `movies` orders alphabetically, but index `movies-2` orders by count.\n - Hint: Remove `federation.mergeFacets` or set `federation.mergeFacets.sortFacetValuesBy` to the desired order.\n Note: index `movies-2` used in `.queries[2]`", + "message": "Inside `.federation.facetsByIndex.movies-2`: Inconsistent order for values in facet `color`: index `movies` orders alphabetically, but index `movies-2` orders by count.\n - Hint: Remove `federation.mergeFacets` or change `faceting.sortFacetValuesBy` to be consistent in settings.\n Note: index `movies-2` used in `.queries[2]`", "code": "invalid_multi_search_facet_order", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_facet_order" From 5de4b48552827616e5a27f93ba8d7bcfa09b0d60 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 17 Sep 2024 17:49:00 +0200 Subject: [PATCH 22/26] Fixup error messages --- meilisearch/src/search/federated.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/meilisearch/src/search/federated.rs b/meilisearch/src/search/federated.rs index 804d56689..7efbea20b 100644 --- a/meilisearch/src/search/federated.rs +++ b/meilisearch/src/search/federated.rs @@ -558,7 +558,7 @@ pub fn perform_federated_search( error.message = format!( "Inside `.federation.facetsByIndex.{index_uid}`: {error}{}", if let Some(query_index) = first_query_index { - format!("\n Note: index `{index_uid}` used in `.queries[{query_index}]`") + format!("\n - Note: index `{index_uid}` used in `.queries[{query_index}]`") } else { Default::default() } @@ -755,7 +755,7 @@ pub fn perform_federated_search( "Inside `.federation.facetsByIndex.{index_uid}`: {}{}", error.message, if let Some(query_index) = first_query_index { - format!("\n Note: index `{index_uid}` used in `.queries[{query_index}]`") + format!("\n - Note: index `{index_uid}` used in `.queries[{query_index}]`") } else { Default::default() } @@ -783,7 +783,7 @@ pub fn perform_federated_search( // here the resource not found is not part of the URL. err.code = StatusCode::BAD_REQUEST; err.message = format!( - "Inside `.federation.facetsByIndex.{index_uid}`: {}\n Note: index `{index_uid}` is not used in queries", + "Inside `.federation.facetsByIndex.{index_uid}`: {}\n - Note: index `{index_uid}` is not used in queries", err.message ); return Err(err); @@ -797,7 +797,7 @@ pub fn perform_federated_search( check_facet_order(&mut facet_order, &index_uid, &facets, &index, &rtxn) { error.message = format!( - "Inside `.federation.facetsByIndex.{index_uid}`: {error}\n Note: index `{index_uid}` is not used in queries", + "Inside `.federation.facetsByIndex.{index_uid}`: {error}\n - Note: index `{index_uid}` is not used in queries", ); return Err(error); } @@ -813,7 +813,7 @@ pub fn perform_federated_search( super::Route::MultiSearch, ) { error.message = - format!("Inside `.federation.facetsByIndex.{index_uid}`: {}\n Note: index `{index_uid}` is not used in queries", error.message); + format!("Inside `.federation.facetsByIndex.{index_uid}`: {}\n - Note: index `{index_uid}` is not used in queries", error.message); return Err(error); } } From 52a52f97cf1a46b66302323ab646fb1088036bd7 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 17 Sep 2024 17:49:12 +0200 Subject: [PATCH 23/26] Update tests --- meilisearch/tests/search/multi.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/meilisearch/tests/search/multi.rs b/meilisearch/tests/search/multi.rs index 1a2ca4c84..b9593f05f 100644 --- a/meilisearch/tests/search/multi.rs +++ b/meilisearch/tests/search/multi.rs @@ -3988,7 +3988,7 @@ async fn federation_non_faceted_for_an_index() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.federation.facetsByIndex.fruits-no-name`: Invalid facet distribution, attribute `name` is not filterable. The available filterable attributes are `BOOST, id`.\n Note: index `fruits-no-name` used in `.queries[1]`", + "message": "Inside `.federation.facetsByIndex.fruits-no-name`: Invalid facet distribution, attribute `name` is not filterable. The available filterable attributes are `BOOST, id`.\n - Note: index `fruits-no-name` used in `.queries[1]`", "code": "invalid_multi_search_facets", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_facets" @@ -4010,7 +4010,7 @@ async fn federation_non_faceted_for_an_index() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.federation.facetsByIndex.fruits-no-name`: Invalid facet distribution, attribute `name` is not filterable. The available filterable attributes are `BOOST, id`.\n Note: index `fruits-no-name` is not used in queries", + "message": "Inside `.federation.facetsByIndex.fruits-no-name`: Invalid facet distribution, attribute `name` is not filterable. The available filterable attributes are `BOOST, id`.\n - Note: index `fruits-no-name` is not used in queries", "code": "invalid_multi_search_facets", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_facets" @@ -4033,7 +4033,7 @@ async fn federation_non_faceted_for_an_index() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.federation.facetsByIndex.fruits-no-facets`: Invalid facet distribution, this index does not have configured filterable attributes.\n Note: index `fruits-no-facets` is not used in queries", + "message": "Inside `.federation.facetsByIndex.fruits-no-facets`: Invalid facet distribution, this index does not have configured filterable attributes.\n - Note: index `fruits-no-facets` is not used in queries", "code": "invalid_multi_search_facets", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_facets" @@ -4055,7 +4055,7 @@ async fn federation_non_faceted_for_an_index() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.federation.facetsByIndex.zorglub`: Index `zorglub` not found.\n Note: index `zorglub` is not used in queries", + "message": "Inside `.federation.facetsByIndex.zorglub`: Index `zorglub` not found.\n - Note: index `zorglub` is not used in queries", "code": "index_not_found", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#index_not_found" @@ -5797,7 +5797,7 @@ async fn federation_inconsistent_merge_order() { snapshot!(code, @"400 Bad Request"); insta::assert_json_snapshot!(response, { ".processingTimeMs" => "[time]" }, @r###" { - "message": "Inside `.federation.facetsByIndex.movies-2`: Inconsistent order for values in facet `color`: index `movies` orders alphabetically, but index `movies-2` orders by count.\n - Hint: Remove `federation.mergeFacets` or change `faceting.sortFacetValuesBy` to be consistent in settings.\n Note: index `movies-2` used in `.queries[2]`", + "message": "Inside `.federation.facetsByIndex.movies-2`: Inconsistent order for values in facet `color`: index `movies` orders alphabetically, but index `movies-2` orders by count.\n - Hint: Remove `federation.mergeFacets` or change `faceting.sortFacetValuesBy` to be consistent in settings.\n - Note: index `movies-2` used in `.queries[2]`", "code": "invalid_multi_search_facet_order", "type": "invalid_request", "link": "https://docs.meilisearch.com/errors#invalid_multi_search_facet_order" From 174d69ff727c7213d037e5381fe2dd7b077e3de9 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Tue, 17 Sep 2024 18:16:14 +0200 Subject: [PATCH 24/26] Don't override max value in indexes --- meilisearch/src/search/federated.rs | 7 ------ meilisearch/src/search/mod.rs | 35 ++++++++--------------------- 2 files changed, 9 insertions(+), 33 deletions(-) diff --git a/meilisearch/src/search/federated.rs b/meilisearch/src/search/federated.rs index 7efbea20b..94a25a0c9 100644 --- a/meilisearch/src/search/federated.rs +++ b/meilisearch/src/search/federated.rs @@ -496,9 +496,6 @@ pub fn perform_federated_search( // 2. perform queries, merge and make hits index by index let required_hit_count = federation.limit + federation.offset; - let override_max_values_per_facet = - federation.merge_facets.and_then(|merge_facets| merge_facets.max_values_per_facet); - // In step (2), semantic_hit_count will be set to Some(0) if any search kind uses semantic // Then in step (3), we'll update its value if there is any semantic search let mut semantic_hit_count = None; @@ -744,8 +741,6 @@ pub fn perform_federated_search( &index, &rtxn, candidates, - override_max_values_per_facet, - None, super::Route::MultiSearch, ) }) @@ -808,8 +803,6 @@ pub fn perform_federated_search( &index, &rtxn, Default::default(), - override_max_values_per_facet, - None, super::Route::MultiSearch, ) { error.message = diff --git a/meilisearch/src/search/mod.rs b/meilisearch/src/search/mod.rs index 4d5d8d890..5bba40a07 100644 --- a/meilisearch/src/search/mod.rs +++ b/meilisearch/src/search/mod.rs @@ -990,15 +990,7 @@ pub fn perform_search( let (facet_distribution, facet_stats) = facets .map(move |facets| { - compute_facet_distribution_stats( - &facets, - index, - &rtxn, - candidates, - None, - None, - Route::Search, - ) + compute_facet_distribution_stats(&facets, index, &rtxn, candidates, Route::Search) }) .transpose()? .map(|ComputedFacets { distribution, stats }| (distribution, stats)) @@ -1034,39 +1026,30 @@ fn compute_facet_distribution_stats>( index: &Index, rtxn: &RoTxn, candidates: roaring::RoaringBitmap, - override_max_values_per_facet: Option, - override_sort_facet_values_by: Option, route: Route, ) -> Result { let mut facet_distribution = index.facets_distribution(rtxn); - let max_values_by_facet = match override_max_values_per_facet { - Some(max_values_by_facet) => max_values_by_facet, - None => index - .max_values_per_facet(rtxn) - .map_err(milli::Error::from)? - .map(|x| x as usize) - .unwrap_or(DEFAULT_VALUES_PER_FACET), - }; + let max_values_by_facet = index + .max_values_per_facet(rtxn) + .map_err(milli::Error::from)? + .map(|x| x as usize) + .unwrap_or(DEFAULT_VALUES_PER_FACET); facet_distribution.max_values_per_facet(max_values_by_facet); let sort_facet_values_by = index.sort_facet_values_by(rtxn).map_err(milli::Error::from)?; - let sort_facet_values_by = |n: &str| match override_sort_facet_values_by { - Some(order_by) => order_by, - None => sort_facet_values_by.get(n), - }; - // add specific facet if there is no placeholder if facets.iter().all(|f| f.as_ref() != "*") { - let fields: Vec<_> = facets.iter().map(|n| (n, sort_facet_values_by(n.as_ref()))).collect(); + let fields: Vec<_> = + facets.iter().map(|n| (n, sort_facet_values_by.get(n.as_ref()))).collect(); facet_distribution.facets(fields); } let distribution = facet_distribution .candidates(candidates) - .default_order_by(sort_facet_values_by("*")) + .default_order_by(sort_facet_values_by.get("*")) .execute() .map_err(|error| match (error, route) { ( From c2caff1716a84a93c5652180ed2ae95c325acd76 Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Wed, 18 Sep 2024 11:26:43 +0200 Subject: [PATCH 25/26] Remove obsolete enum --- meilisearch/src/search/federated.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/meilisearch/src/search/federated.rs b/meilisearch/src/search/federated.rs index 94a25a0c9..170da4112 100644 --- a/meilisearch/src/search/federated.rs +++ b/meilisearch/src/search/federated.rs @@ -90,14 +90,6 @@ pub struct MergeFacets { pub max_values_per_facet: Option, } -#[derive(Debug, deserr::Deserr, Default)] -#[deserr(rename_all = camelCase, deny_unknown_fields)] -pub enum GroupFacetsBy { - Facet, - #[default] - Index, -} - #[derive(Debug, deserr::Deserr)] #[deserr(error = DeserrJsonError, rename_all = camelCase, deny_unknown_fields)] pub struct FederatedSearch { From 00f8d03f4349888b654456aa2cc2683aefffaece Mon Sep 17 00:00:00 2001 From: Louis Dureuil Date: Wed, 18 Sep 2024 11:43:07 +0200 Subject: [PATCH 26/26] Use f32::min and f32::max --- meilisearch/src/search/federated.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/meilisearch/src/search/federated.rs b/meilisearch/src/search/federated.rs index 170da4112..5279c26bb 100644 --- a/meilisearch/src/search/federated.rs +++ b/meilisearch/src/search/federated.rs @@ -408,10 +408,8 @@ impl FederatedFacets { std::collections::btree_map::Entry::Occupied(mut entry) => { let stats = entry.get_mut(); - stats.min = - if stats.min <= index_stats.min { stats.min } else { index_stats.min }; - stats.max = - if stats.max >= index_stats.max { stats.max } else { index_stats.max }; + stats.min = f64::min(stats.min, index_stats.min); + stats.max = f64::max(stats.max, index_stats.max); } } }