recreate most filter error except for the geosearch

filter_parser/src/error.rs

@@ -71,6 +71,10 @@ impl<'a> Error<'a> {
         Self { context, kind }
     }
 
+    pub fn new_from_external(context: Span<'a>, error: impl std::error::Error) -> Self {
+        Self::new_from_kind(context, ErrorKind::External(error.to_string()))
+    }
+
     pub fn char(self) -> char {
         match self.kind {
             ErrorKind::Char(c) => c,

filter_parser/src/lib.rs

@@ -69,6 +69,10 @@ impl<'a> Token<'a> {
     pub fn new(position: Span<'a>) -> Self {
         Self { position, inner: &position }
     }
+
+    pub fn as_external_error(&self, error: impl std::error::Error) -> Error<'a> {
+        Error::new_from_external(self.position, error)
+    }
 }
 
 impl<'a> From<Span<'a>> for Token<'a> {

milli/src/error.rs

@@ -59,7 +59,7 @@ pub enum UserError {
     InvalidDocumentId { document_id: Value },
     InvalidFacetsDistribution { invalid_facets_name: HashSet<String> },
     InvalidGeoField { document_id: Value, object: Value },
-    InvalidFilter { input: String },
+    InvalidFilter(String),
     InvalidSortName { name: String },
     InvalidSortableAttribute { field: String, valid_fields: HashSet<String> },
     SortRankingRuleMissing,
@@ -207,7 +207,7 @@ impl StdError for InternalError {}
 impl fmt::Display for UserError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            Self::InvalidFilter { input } => write!(f, "parser error {}", input),
+            Self::InvalidFilter(input) => write!(f, "{}", input),
             Self::AttributeLimitReached => f.write_str("maximum number of attributes reached"),
             Self::CriterionError(error) => write!(f, "{}", error),
             Self::DocumentLimitReached => f.write_str("maximum number of documents reached"),

milli/src/search/facet/filter_condition.rs

@@ -1,4 +1,4 @@
-use std::fmt::Debug;
+use std::fmt::{Debug, Display};
 use std::ops::Bound::{self, Excluded, Included};
 use std::str::FromStr;
 
@@ -20,18 +20,50 @@ pub struct Filter<'a> {
     condition: FilterCondition<'a>,
 }
 
-fn parse<T: FromStr>(tok: &Token) -> Result<T> {
+#[derive(Debug)]
+enum FilterError<'a> {
+    AttributeNotFilterable { attribute: &'a str, filterable: String },
+    BadGeo(&'a str),
+    Reserved(&'a str),
+}
+impl<'a> std::error::Error for FilterError<'a> {}
+
+impl<'a> Display for FilterError<'a> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::AttributeNotFilterable { attribute, filterable } => write!(
+                f,
+                "Attribute `{}` is not filterable. Available filterable attributes are: `{}`.",
+                attribute,
+                filterable,
+            ),
+            Self::Reserved(keyword) => write!(
+                f,
+                "`{}` is a reserved keyword and thus can't be used as a filter expression.",
+                keyword
+            ),
+            Self::BadGeo(keyword) => write!(f, "`{}` is a reserved keyword and thus can't be used as a filter expression. Use the _geoRadius(latitude, longitude, distance) built-in rule to filter on _geo field coordinates.", keyword),
+        }
+    }
+}
+
+impl<'a> From<FPError<'a>> for Error {
+    fn from(error: FPError<'a>) -> Self {
+        Self::UserError(UserError::InvalidFilter(error.to_string()))
+    }
+}
+
+fn parse<T>(tok: &Token) -> Result<T>
+where
+    T: FromStr,
+    T::Err: std::error::Error,
+{
     match tok.inner.parse::<T>() {
         Ok(t) => Ok(t),
-        Err(_e) => Err(UserError::InvalidFilter {
+        Err(e) => {
-            input: format!(
+            Err(UserError::InvalidFilter(FPError::new_from_external(tok.position, e).to_string())
-                "Could not parse `{}` at line {} and offset {}",
+                .into())
-                tok.inner,
-                tok.position.location_line(),
-                tok.position.get_column()
-            ),
         }
-        .into()),
     }
 }
 
@@ -90,7 +122,7 @@ impl<'a> Filter<'a> {
     pub fn from_str(expression: &'a str) -> Result<Self> {
         let condition = match FilterCondition::parse(expression) {
             Ok(fc) => Ok(fc),
-            Err(e) => Err(Error::UserError(UserError::InvalidFilter { input: e.to_string() })),
+            Err(e) => Err(Error::UserError(UserError::InvalidFilter(e.to_string()))),
         }?;
         Ok(Self { condition })
     }
@@ -299,25 +331,26 @@ impl<'a> Filter<'a> {
                     Self::evaluate_operator(rtxn, index, numbers_db, strings_db, fid, &op)
                 } else {
                     match fid.inner {
-                        // TODO update the error messages according to the spec
+                        attribute @ "_geo" => {
-                        "_geo" => {
+                            return Err(fid.as_external_error(FilterError::BadGeo(attribute)))?;
-                            return Err(UserError::InvalidFilter { input: format!("Tried to use _geo in a filter, you probably wanted to use _geoRadius(latitude, longitude, radius)") })?;
                         }
-                        "_geoDistance" => {
+                        attribute if attribute.starts_with("_geoPoint(") => {
-                            return Err(UserError::InvalidFilter {
+                            return Err(fid.as_external_error(FilterError::BadGeo("_geoPoint")))?;
-                                input: format!("Reserved field _geoDistance"),
-                            })?;
                         }
-                        fid if fid.starts_with("_geoPoint(") => {
+                        attribute @ "_geoDistance" => {
-                            return Err(UserError::InvalidFilter { input: format!("_geoPoint only available in sort. You wanted to use _geoRadius") })?;
+                            return Err(fid.as_external_error(FilterError::Reserved(attribute)))?;
                         }
-                        fid => {
+                        attribute => {
-                            return Err(UserError::InvalidFilter {
+                            return Err(fid.as_external_error(
-                                input: format!(
+                                FilterError::AttributeNotFilterable {
-                                    "Bad filter {}, available filters are {:?}",
+                                    attribute,
-                                    fid, filterable_fields
+                                    filterable: filterable_fields
-                                ),
+                                        .iter()
-                            })?;
+                                        .map(|(_, s)| s)
+                                        .collect::<Vec<_>>()
+                                        .join(" "),
+                                },
+                            ))?;
                         }
                     }
                 }
@@ -356,9 +389,9 @@ impl<'a> Filter<'a> {
                     Ok(result)
                 } else {
                     // TODO TAMO: update the error message
-                    return Err(UserError::InvalidFilter {
+                    return Err(UserError::InvalidFilter(format!(
-                        input: format!("You tried to use _geo in a filter, you probably wanted to use _geoRadius"),
+                        "You tried to use _geo in a filter, you probably wanted to use _geoRadius"
-                    })?;
+                    )))?;
                 }
             }
             FilterCondition::GeoGreaterThan { point, radius } => {