diff --git a/milli/src/error.rs b/milli/src/error.rs index d927407f0..713935869 100644 --- a/milli/src/error.rs +++ b/milli/src/error.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::convert::Infallible; use std::error::Error as StdError; use std::{fmt, io, str}; @@ -51,13 +52,14 @@ pub enum FieldIdMapMissingEntry { pub enum UserError { AttributeLimitReached, Csv(csv::Error), - MaxDatabaseSizeReached, DocumentLimitReached, - InvalidFilter(pest::error::Error), InvalidCriterionName { name: String }, InvalidDocumentId { document_id: Value }, + InvalidFacetsDistribution { invalid_facets_name: HashSet }, + InvalidFilter(pest::error::Error), InvalidFilterAttribute(pest::error::Error), InvalidStoreFile, + MaxDatabaseSizeReached, MissingDocumentId { document: Object }, MissingPrimaryKey, NoSpaceLeftOnDevice, @@ -202,6 +204,15 @@ impl fmt::Display for UserError { Self::AttributeLimitReached => f.write_str("maximum number of attributes reached"), Self::Csv(error) => error.fmt(f), Self::DocumentLimitReached => f.write_str("maximum number of documents reached"), + Self::InvalidFacetsDistribution { invalid_facets_name } => { + let name_list = + invalid_facets_name.iter().map(AsRef::as_ref).collect::>().join(", "); + write!( + f, + "invalid facet distribution, the fields {} are not set as filterable", + name_list + ) + } Self::InvalidFilter(error) => error.fmt(f), Self::InvalidCriterionName { name } => write!(f, "invalid criterion {}", name), Self::InvalidDocumentId { document_id } => { diff --git a/milli/src/search/facet/facet_distribution.rs b/milli/src/search/facet/facet_distribution.rs index 0a2036494..71816cf5d 100644 --- a/milli/src/search/facet/facet_distribution.rs +++ b/milli/src/search/facet/facet_distribution.rs @@ -6,7 +6,7 @@ use heed::types::{ByteSlice, Unit}; use heed::{BytesDecode, Database}; use roaring::RoaringBitmap; -use crate::error::FieldIdMapMissingEntry; +use crate::error::{FieldIdMapMissingEntry, UserError}; use crate::facet::FacetType; use crate::heed_codec::facet::FacetValueStringCodec; use crate::search::facet::{FacetIter, FacetRange}; @@ -174,49 +174,64 @@ impl<'a> FacetDistribution<'a> { fn facet_values(&self, field_id: FieldId) -> heed::Result> { use FacetType::{Number, String}; - if let Some(candidates) = self.candidates.as_ref() { - // Classic search, candidates were specified, we must return facet values only related - // to those candidates. We also enter here for facet strings for performance reasons. - let mut distribution = BTreeMap::new(); - if candidates.len() <= CANDIDATES_THRESHOLD { - self.facet_distribution_from_documents( - field_id, - Number, - candidates, - &mut distribution, - )?; - self.facet_distribution_from_documents( - field_id, - String, - candidates, - &mut distribution, - )?; - } else { - self.facet_numbers_distribution_from_facet_levels( - field_id, - candidates, - &mut distribution, - )?; - self.facet_distribution_from_documents( - field_id, - String, - candidates, - &mut distribution, - )?; - } + match self.candidates { + Some(ref candidates) => { + // Classic search, candidates were specified, we must return facet values only related + // to those candidates. We also enter here for facet strings for performance reasons. + let mut distribution = BTreeMap::new(); + if candidates.len() <= CANDIDATES_THRESHOLD { + self.facet_distribution_from_documents( + field_id, + Number, + candidates, + &mut distribution, + )?; + self.facet_distribution_from_documents( + field_id, + String, + candidates, + &mut distribution, + )?; + } else { + self.facet_numbers_distribution_from_facet_levels( + field_id, + candidates, + &mut distribution, + )?; + self.facet_distribution_from_documents( + field_id, + String, + candidates, + &mut distribution, + )?; + } - Ok(distribution) - } else { - self.facet_values_from_raw_facet_database(field_id) + Ok(distribution) + } + None => self.facet_values_from_raw_facet_database(field_id), } } pub fn execute(&self) -> Result>> { let fields_ids_map = self.index.fields_ids_map(self.rtxn)?; let filterable_fields = self.index.filterable_fields(self.rtxn)?; + let fields = match self.facets { + Some(ref facets) => { + let invalid_fields: HashSet<_> = facets.difference(&filterable_fields).collect(); + if !invalid_fields.is_empty() { + return Err(UserError::InvalidFacetsDistribution { + invalid_facets_name: invalid_fields.into_iter().cloned().collect(), + } + .into()); + } else { + facets.clone() + } + } + None => filterable_fields, + }; let mut distribution = BTreeMap::new(); - for name in filterable_fields { + for name in fields { let fid = fields_ids_map.id(&name).ok_or_else(|| FieldIdMapMissingEntry::FieldName { field_name: name.clone(),