Ditch usage of check_sort_criteria

crates/meilisearch-types/src/error.rs

@@ -467,7 +467,8 @@ impl ErrorCode for milli::Error {
                     UserError::InvalidDistinctAttribute { .. } => Code::InvalidSearchDistinct,
                     UserError::SortRankingRuleMissing => Code::InvalidSearchSort,
                     UserError::InvalidFacetsDistribution { .. } => Code::InvalidSearchFacets,
-                    UserError::InvalidSortableAttribute { .. } => Code::InvalidSearchSort,
+                    UserError::InvalidSearchSortableAttribute { .. } => Code::InvalidSearchSort,
+                    UserError::InvalidDocumentSortableAttribute { .. } => Code::InvalidDocumentSort,
                     UserError::InvalidSearchableAttribute { .. } => {
                         Code::InvalidSearchAttributesToSearchOn
                     }

crates/milli/src/error.rs

@@ -191,7 +191,21 @@ and can not be more than 511 bytes.", .document_id.to_string()
                 ),
         }
     )]
-    InvalidSortableAttribute { field: String, valid_fields: BTreeSet<String>, hidden_fields: bool },
+    InvalidSearchSortableAttribute {
+        field: String,
+        valid_fields: BTreeSet<String>,
+        hidden_fields: bool,
+    },
+    #[error("Attribute `{}` is not sortable. {}",
+        .field,
+        match .sortable_fields.is_empty() {
+            true => "This index does not have configured sortable attributes.".to_string(),
+            false => format!("Available sortable attributes are: `{}`.",
+                    sortable_fields.iter().map(AsRef::as_ref).collect::<Vec<&str>>().join(", ")
+                ),
+        }
+    )]
+    InvalidDocumentSortableAttribute { field: String, sortable_fields: BTreeSet<String> },
     #[error("Attribute `{}` is not filterable and thus, cannot be used as distinct attribute. {}",
         .field,
         match (.valid_patterns.is_empty(), .matching_rule_index) {
@@ -614,7 +628,7 @@ fn conditionally_lookup_for_error_message() {
     ];
 
     for (list, suffix) in messages {
-        let err = UserError::InvalidSortableAttribute {
+        let err = UserError::InvalidSearchSortableAttribute {
             field: "name".to_string(),
             valid_fields: list,
             hidden_fields: false,

crates/milli/src/facet/facet_sort_recursive.rs

@@ -1,16 +1,15 @@
-use std::collections::VecDeque;
+use std::collections::{BTreeSet, VecDeque};
 
 use crate::{
+    constants::RESERVED_GEO_FIELD_NAME,
     documents::{geo_sort::next_bucket, GeoSortParameter},
     heed_codec::{
         facet::{FacetGroupKeyCodec, FacetGroupValueCodec},
         BytesRefCodec,
     },
-    search::{
+    is_faceted,
-        facet::{ascending_facet_sort, descending_facet_sort},
+    search::facet::{ascending_facet_sort, descending_facet_sort},
-        new::check_sort_criteria,
+    AscDesc, DocumentId, Member, UserError,
-    },
-    AscDesc, DocumentId, Member,
 };
 use heed::Database;
 use roaring::RoaringBitmap;
@@ -366,49 +365,47 @@ pub fn recursive_facet_sort<'ctx>(
     sort: Vec<AscDesc>,
     candidates: &'ctx RoaringBitmap,
 ) -> crate::Result<SortedDocuments<'ctx>> {
-    check_sort_criteria(index, rtxn, Some(&sort))?;
+    let sortable_fields: BTreeSet<_> = index.sortable_fields(rtxn)?.into_iter().collect();
-
-    let mut fields = Vec::new();
     let fields_ids_map = index.fields_ids_map(rtxn)?;
+    
+    let mut fields = Vec::new();
     let mut need_geo_candidates = false;
-    for sort in sort {
+    for asc_desc in sort {
-        match sort {
+        let (field, geofield) = match asc_desc {
-            AscDesc::Asc(Member::Field(field)) => {
+            AscDesc::Asc(Member::Field(field)) => (Some((field, true)), None),
-                if let Some(field_id) = fields_ids_map.id(&field) {
+            AscDesc::Desc(Member::Field(field)) => (Some((field, false)), None),
-                    fields.push(AscDescId::Facet { field_id, ascending: true });
+            AscDesc::Asc(Member::Geo(target_point)) => (None, Some((target_point, true))),
-                }
+            AscDesc::Desc(Member::Geo(target_point)) => (None, Some((target_point, false))),
-            }
-            AscDesc::Desc(Member::Field(field)) => {
-                if let Some(field_id) = fields_ids_map.id(&field) {
-                    fields.push(AscDescId::Facet { field_id, ascending: false });
-                }
-            }
-            AscDesc::Asc(Member::Geo(target_point)) => {
-                if let (Some(lat), Some(lng)) =
-                    (fields_ids_map.id("_geo.lat"), fields_ids_map.id("_geo.lng"))
-                {
-                    need_geo_candidates = true;
-                    fields.push(AscDescId::Geo {
-                        field_ids: [lat, lng],
-                        target_point,
-                        ascending: true,
-                    });
-                }
-            }
-            AscDesc::Desc(Member::Geo(target_point)) => {
-                if let (Some(lat), Some(lng)) =
-                    (fields_ids_map.id("_geo.lat"), fields_ids_map.id("_geo.lng"))
-                {
-                    need_geo_candidates = true;
-                    fields.push(AscDescId::Geo {
-                        field_ids: [lat, lng],
-                        target_point,
-                        ascending: false,
-                    });
-                }
-            }
         };
-        // FIXME: Should this return an error if the field is not found?
+        if let Some((field, ascending)) = field {
+            if is_faceted(&field, &sortable_fields) {
+                if let Some(field_id) = fields_ids_map.id(&field) {
+                    fields.push(AscDescId::Facet { field_id, ascending });
+                    continue;
+                }
+            }
+            return Err(UserError::InvalidDocumentSortableAttribute {
+                field: field.to_string(),
+                sortable_fields: sortable_fields.clone(),
+            }
+            .into());
+        }
+        if let Some((target_point, ascending)) = geofield {
+            if sortable_fields.contains(RESERVED_GEO_FIELD_NAME) {
+                if let (Some(lat), Some(lng)) =
+                    (fields_ids_map.id("_geo.lat"), fields_ids_map.id("_geo.lng"))
+                {
+                    need_geo_candidates = true;
+                    fields.push(AscDescId::Geo { field_ids: [lat, lng], target_point, ascending });
+                    continue;
+                }
+            }
+            return Err(UserError::InvalidDocumentSortableAttribute {
+                field: RESERVED_GEO_FIELD_NAME.to_string(),
+                sortable_fields: sortable_fields.clone(),
+            }
+            .into());
+        }
     }
 
     let geo_candidates = if need_geo_candidates {

crates/milli/src/search/new/mod.rs

@@ -903,7 +903,7 @@ pub(crate) fn check_sort_criteria(
                 let (valid_fields, hidden_fields) =
                     index.remove_hidden_fields(rtxn, sortable_fields)?;
 
-                return Err(UserError::InvalidSortableAttribute {
+                return Err(UserError::InvalidSearchSortableAttribute {
                     field: field.to_string(),
                     valid_fields,
                     hidden_fields,
@@ -914,7 +914,7 @@ pub(crate) fn check_sort_criteria(
                 let (valid_fields, hidden_fields) =
                     index.remove_hidden_fields(rtxn, sortable_fields)?;
 
-                return Err(UserError::InvalidSortableAttribute {
+                return Err(UserError::InvalidSearchSortableAttribute {
                     field: RESERVED_GEO_FIELD_NAME.to_string(),
                     valid_fields,
                     hidden_fields,