Remove runtime sortFacetValuesBy

This commit is contained in:
Louis Dureuil 2024-09-17 17:22:03 +02:00
parent d9e0df74ea
commit 98b77aec66
No known key found for this signature in database
2 changed files with 21 additions and 61 deletions

View File

@ -248,7 +248,6 @@ InvalidMultiSearchMergeFacets , InvalidRequest , BAD_REQUEST ;
InvalidMultiSearchQueryFacets , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchQueryFacets , InvalidRequest , BAD_REQUEST ;
InvalidMultiSearchQueryPagination , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchQueryPagination , InvalidRequest , BAD_REQUEST ;
InvalidMultiSearchQueryRankingRules , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchQueryRankingRules , InvalidRequest , BAD_REQUEST ;
InvalidMultiSearchSortFacetValuesBy , InvalidRequest , BAD_REQUEST ;
InvalidMultiSearchWeight , InvalidRequest , BAD_REQUEST ; InvalidMultiSearchWeight , InvalidRequest , BAD_REQUEST ;
InvalidSearchAttributesToSearchOn , InvalidRequest , BAD_REQUEST ; InvalidSearchAttributesToSearchOn , InvalidRequest , BAD_REQUEST ;
InvalidSearchAttributesToCrop , InvalidRequest , BAD_REQUEST ; InvalidSearchAttributesToCrop , InvalidRequest , BAD_REQUEST ;

View File

@ -13,8 +13,8 @@ use indexmap::IndexMap;
use meilisearch_types::deserr::DeserrJsonError; use meilisearch_types::deserr::DeserrJsonError;
use meilisearch_types::error::deserr_codes::{ use meilisearch_types::error::deserr_codes::{
InvalidMultiSearchFacetsByIndex, InvalidMultiSearchMaxValuesPerFacet, InvalidMultiSearchFacetsByIndex, InvalidMultiSearchMaxValuesPerFacet,
InvalidMultiSearchMergeFacets, InvalidMultiSearchSortFacetValuesBy, InvalidMultiSearchWeight, InvalidMultiSearchMergeFacets, InvalidMultiSearchWeight, InvalidSearchLimit,
InvalidSearchLimit, InvalidSearchOffset, InvalidSearchOffset,
}; };
use meilisearch_types::error::ResponseError; use meilisearch_types::error::ResponseError;
use meilisearch_types::index_uid::IndexUid; use meilisearch_types::index_uid::IndexUid;
@ -86,44 +86,10 @@ pub struct Federation {
#[derive(Copy, Clone, Debug, deserr::Deserr, Default)] #[derive(Copy, Clone, Debug, deserr::Deserr, Default)]
#[deserr(error = DeserrJsonError<InvalidMultiSearchMergeFacets>, rename_all = camelCase, deny_unknown_fields)] #[deserr(error = DeserrJsonError<InvalidMultiSearchMergeFacets>, rename_all = camelCase, deny_unknown_fields)]
pub struct MergeFacets { pub struct MergeFacets {
#[deserr(default, error = DeserrJsonError<InvalidMultiSearchSortFacetValuesBy>)]
pub sort_facet_values_by: SortFacetValuesBy,
#[deserr(default, error = DeserrJsonError<InvalidMultiSearchMaxValuesPerFacet>)] #[deserr(default, error = DeserrJsonError<InvalidMultiSearchMaxValuesPerFacet>)]
pub max_values_per_facet: Option<usize>, pub max_values_per_facet: Option<usize>,
} }
impl MergeFacets {
pub fn to_components(this: Option<Self>) -> (Option<OrderBy>, Option<usize>) {
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<SortFacetValuesBy> for Option<OrderBy> {
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)] #[derive(Debug, deserr::Deserr, Default)]
#[deserr(rename_all = camelCase, deny_unknown_fields)] #[deserr(rename_all = camelCase, deny_unknown_fields)]
pub enum GroupFacetsBy { pub enum GroupFacetsBy {
@ -413,8 +379,8 @@ impl FederatedFacets {
pub fn merge( pub fn merge(
self, self,
MergeFacets { sort_facet_values_by, max_values_per_facet }: MergeFacets, MergeFacets { max_values_per_facet }: MergeFacets,
facet_order: Option<BTreeMap<String, (String, OrderBy)>>, facet_order: BTreeMap<String, (String, OrderBy)>,
) -> Option<ComputedFacets> { ) -> Option<ComputedFacets> {
if self.is_empty() { if self.is_empty() {
return None; return None;
@ -461,12 +427,7 @@ impl FederatedFacets {
// fixup order // fixup order
for (facet, values) in &mut distribution { for (facet, values) in &mut distribution {
let order_by = Option::<OrderBy>::from(sort_facet_values_by) let order_by = facet_order.get(facet).map(|(_, order)| *order).unwrap_or_default();
.or_else(|| match &facet_order {
Some(facet_order) => facet_order.get(facet).map(|(_, order)| *order),
None => None,
})
.unwrap_or_default();
match order_by { match order_by {
OrderBy::Lexicographic => { OrderBy::Lexicographic => {
@ -535,8 +496,8 @@ pub fn perform_federated_search(
// 2. perform queries, merge and make hits index by index // 2. perform queries, merge and make hits index by index
let required_hit_count = federation.limit + federation.offset; let required_hit_count = federation.limit + federation.offset;
let (override_sort_facet_values_by, override_max_values_per_facet) = let override_max_values_per_facet =
MergeFacets::to_components(federation.merge_facets); 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 // 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 // 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. // to detect if the order is inconsistent for a facet.
let mut facet_order: Option<BTreeMap<String, (String, OrderBy)>> = match federation.merge_facets let mut facet_order: Option<BTreeMap<String, (String, OrderBy)>> = match federation.merge_facets
{ {
Some(MergeFacets { sort_facet_values_by: SortFacetValuesBy::IndexSettings, .. }) => { Some(MergeFacets { .. }) => Some(Default::default()),
Some(Default::default())
}
_ => None, _ => None,
}; };
@ -786,7 +745,7 @@ pub fn perform_federated_search(
&rtxn, &rtxn,
candidates, candidates,
override_max_values_per_facet, override_max_values_per_facet,
override_sort_facet_values_by, None,
super::Route::MultiSearch, super::Route::MultiSearch,
) )
}) })
@ -850,7 +809,7 @@ pub fn perform_federated_search(
&rtxn, &rtxn,
Default::default(), Default::default(),
override_max_values_per_facet, override_max_values_per_facet,
override_sort_facet_values_by, None,
super::Route::MultiSearch, super::Route::MultiSearch,
) { ) {
error.message = error.message =
@ -905,17 +864,19 @@ pub fn perform_federated_search(
.map(|hit| hit.hit) .map(|hit| hit.hit)
.collect(); .collect();
let (facet_distribution, facet_stats, facets_by_index) = match federation.merge_facets { let (facet_distribution, facet_stats, facets_by_index) =
Some(merge_facets) => { match federation.merge_facets.zip(facet_order) {
let facets = facets.merge(merge_facets, facet_order); Some((merge_facets, facet_order)) => {
let facets = facets.merge(merge_facets, facet_order);
let (facet_distribution, facet_stats) = let (facet_distribution, facet_stats) = facets
facets.map(|ComputedFacets { distribution, stats }| (distribution, stats)).unzip(); .map(|ComputedFacets { distribution, stats }| (distribution, stats))
.unzip();
(facet_distribution, facet_stats, FederatedFacets::default()) (facet_distribution, facet_stats, FederatedFacets::default())
} }
None => (None, None, facets), None => (None, None, facets),
}; };
let search_result = FederatedSearchResult { let search_result = FederatedSearchResult {
hits: merged_hits, hits: merged_hits,