Merge #341

341: Throw a query time error when a sort parameter is used but the sort ranking rule is missing r=Kerollmops a=Kerollmops

This PR makes the engine throw an error for when the ranking rules don't contain the `sort` rule, the `sortable_fields` are correctly set but the user tries to use the `sort` query parameter. Doing so will have no effect on the returned documents so we preferred returning an error to help debug this.

That's breaking on the MeiliSearch side as we added a new variant to the `UserError` enum.

Co-authored-by: Kerollmops <clement@meilisearch.com>

milli/src/error.rs

@@ -59,6 +59,7 @@ pub enum UserError {
     InvalidFilter(pest::error::Error<ParserRule>),
     InvalidFilterAttribute(pest::error::Error<ParserRule>),
     InvalidSortableAttribute { field: String, valid_fields: HashSet<String> },
+    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)

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,

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";
 

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))));
+}