diff --git a/milli/src/criterion.rs b/milli/src/criterion.rs index 38162a74b..47eb7c7dc 100644 --- a/milli/src/criterion.rs +++ b/milli/src/criterion.rs @@ -64,6 +64,15 @@ pub enum AscDesc { Desc(String), } +impl AscDesc { + pub fn field(&self) -> &str { + match self { + AscDesc::Asc(field) => field, + AscDesc::Desc(field) => field, + } + } +} + impl FromStr for AscDesc { type Err = UserError; diff --git a/milli/src/error.rs b/milli/src/error.rs index 713935869..9bda74631 100644 --- a/milli/src/error.rs +++ b/milli/src/error.rs @@ -58,6 +58,7 @@ pub enum UserError { InvalidFacetsDistribution { invalid_facets_name: HashSet }, InvalidFilter(pest::error::Error), InvalidFilterAttribute(pest::error::Error), + InvalidSortableAttribute { field: String, valid_fields: HashSet }, InvalidStoreFile, MaxDatabaseSizeReached, MissingDocumentId { document: Object }, @@ -226,6 +227,15 @@ only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and undersco ) } Self::InvalidFilterAttribute(error) => error.fmt(f), + Self::InvalidSortableAttribute { field, valid_fields } => { + let valid_names = + valid_fields.iter().map(AsRef::as_ref).collect::>().join(", "); + write!( + f, + "Attribute {} is not sortable, available sortable attributes are: {}", + field, valid_names + ) + } Self::MissingDocumentId { document } => { let json = serde_json::to_string(document).unwrap(); write!(f, "document doesn't have an identifier {}", json) diff --git a/milli/src/search/mod.rs b/milli/src/search/mod.rs index ce2efcc98..23e5c1834 100644 --- a/milli/src/search/mod.rs +++ b/milli/src/search/mod.rs @@ -19,6 +19,7 @@ pub use self::facet::{FacetDistribution, FacetNumberIter, FilterCondition, Opera pub use self::matching_words::MatchingWords; use self::query_tree::QueryTreeBuilder; use crate::criterion::AscDesc; +use crate::error::UserError; use crate::search::criteria::r#final::{Final, FinalResult}; use crate::{DocumentId, Index, Result}; @@ -142,13 +143,28 @@ impl<'a> Search<'a> { None => MatchingWords::default(), }; + // We check that we are allowed to use the sort criteria, we check + // that they are declared in the sortable fields. + let sortable_fields = self.index.sortable_fields(self.rtxn)?; + if let Some(sort_criteria) = &self.sort_criteria { + for asc_desc in sort_criteria { + let field = asc_desc.field(); + if !sortable_fields.contains(field) { + return Err(UserError::InvalidSortableAttribute { + field: field.to_string(), + valid_fields: sortable_fields, + } + .into()); + } + } + } + let criteria_builder = criteria::CriteriaBuilder::new(self.rtxn, self.index)?; - let sort_criteria = self.sort_criteria.clone(); let criteria = criteria_builder.build( query_tree, primitive_query, filtered_candidates, - sort_criteria, + self.sort_criteria.clone(), )?; match self.index.distinct_field(self.rtxn)? {