From 8dca36433c67c862b1b41ce68695d05a00f75dcb Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Tue, 7 Sep 2021 10:37:57 +0200 Subject: [PATCH 1/3] Introduce the new SortRankingRuleMissing user error variant --- milli/src/error.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/milli/src/error.rs b/milli/src/error.rs index 9bda74631..56028f742 100644 --- a/milli/src/error.rs +++ b/milli/src/error.rs @@ -59,6 +59,7 @@ pub enum UserError { InvalidFilter(pest::error::Error), InvalidFilterAttribute(pest::error::Error), InvalidSortableAttribute { field: String, valid_fields: HashSet }, + SortRankingRuleMissing, InvalidStoreFile, MaxDatabaseSizeReached, MissingDocumentId { document: Object }, @@ -236,6 +237,10 @@ only composed of alphanumeric characters (a-z A-Z 0-9), hyphens (-) and undersco field, valid_names ) } + Self::SortRankingRuleMissing => f.write_str( + "The sort ranking rule must be specified in the \ + ranking rules settings to use the sort parameter at search time", + ), Self::MissingDocumentId { document } => { let json = serde_json::to_string(document).unwrap(); write!(f, "document doesn't have an identifier {}", json) From fd3daa442362d23d9db477665faa6437fdb77593 Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Tue, 7 Sep 2021 10:52:19 +0200 Subject: [PATCH 2/3] Throw a query time error when a sort param is used but sort ranking rule is missing --- milli/src/search/mod.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/milli/src/search/mod.rs b/milli/src/search/mod.rs index 56002b2e3..207f46f8a 100644 --- a/milli/src/search/mod.rs +++ b/milli/src/search/mod.rs @@ -18,7 +18,7 @@ pub(crate) use self::facet::ParserRule; pub use self::facet::{FacetDistribution, FacetNumberIter, FilterCondition, Operator}; pub use self::matching_words::MatchingWords; use self::query_tree::QueryTreeBuilder; -use crate::criterion::AscDesc; +use crate::criterion::{AscDesc, Criterion}; use crate::error::UserError; use crate::search::criteria::r#final::{Final, FinalResult}; use crate::{DocumentId, Index, Result}; @@ -159,6 +159,14 @@ impl<'a> Search<'a> { } } + // We check that the sort ranking rule exists and throw an + // error if we try to use it and that it doesn't. + let sort_ranking_rule_missing = !self.index.criteria(self.rtxn)?.contains(&Criterion::Sort); + let empty_sort_criteria = self.sort_criteria.as_ref().map_or(true, |s| s.is_empty()); + if sort_ranking_rule_missing && !empty_sort_criteria { + return Err(UserError::SortRankingRuleMissing.into()); + } + let criteria_builder = criteria::CriteriaBuilder::new(self.rtxn, self.index)?; let criteria = criteria_builder.build( query_tree, From 5989528833d1f70baf49a288f566a09555ff052c Mon Sep 17 00:00:00 2001 From: Kerollmops Date: Tue, 7 Sep 2021 11:01:37 +0200 Subject: [PATCH 3/3] Add a test to make sure we throw the right error message --- milli/tests/search/mod.rs | 1 + milli/tests/search/sort.rs | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 milli/tests/search/sort.rs diff --git a/milli/tests/search/mod.rs b/milli/tests/search/mod.rs index 0fbc0e1b6..c34434c4e 100644 --- a/milli/tests/search/mod.rs +++ b/milli/tests/search/mod.rs @@ -13,6 +13,7 @@ use slice_group_by::GroupBy; mod distinct; mod filters; mod query_criteria; +mod sort; pub const TEST_QUERY: &'static str = "hello world america"; diff --git a/milli/tests/search/sort.rs b/milli/tests/search/sort.rs new file mode 100644 index 000000000..fe87f0623 --- /dev/null +++ b/milli/tests/search/sort.rs @@ -0,0 +1,23 @@ +use big_s::S; +use milli::Criterion::{Attribute, Exactness, Proximity, Typo, Words}; +use milli::{AscDesc, Error, Search, UserError}; + +use crate::search::{self, EXTERNAL_DOCUMENTS_IDS}; + +#[test] +fn sort_ranking_rule_missing() { + let criteria = vec![Words, Typo, Proximity, Attribute, Exactness]; + // sortables: `tag` and `asc_desc_rank` + let index = search::setup_search_index_with_criteria(&criteria); + let rtxn = index.read_txn().unwrap(); + + let mut search = Search::new(&rtxn, &index); + search.query(search::TEST_QUERY); + search.limit(EXTERNAL_DOCUMENTS_IDS.len()); + search.authorize_typos(true); + search.optional_words(true); + search.sort_criteria(vec![AscDesc::Asc(S("tag"))]); + + let result = search.execute(); + assert!(matches!(result, Err(Error::UserError(UserError::SortRankingRuleMissing)))); +}