Add error handling and earth lap collision with bounding box

filter-parser/src/error.rs

@@ -57,8 +57,10 @@ pub enum ExpectedValueKind {
 #[derive(Debug)]
 pub enum ErrorKind<'a> {
     ReservedGeo(&'a str),
-    Geo,
+    GeoRadius,
-    MisusedGeo,
+    GeoBoundingBox,
+    MisusedGeoRadius,
+    MisusedGeoBoundingBox,
     InvalidPrimary,
     ExpectedEof,
     ExpectedValue(ExpectedValueKind),
@@ -150,15 +152,21 @@ impl<'a> Display for Error<'a> {
             ErrorKind::ExpectedEof => {
                 writeln!(f, "Found unexpected characters at the end of the filter: `{}`. You probably forgot an `OR` or an `AND` rule.", escaped_input)?
             }
-            ErrorKind::Geo => {
+            ErrorKind::GeoRadius => {
                 writeln!(f, "The `_geoRadius` filter expects three arguments: `_geoRadius(latitude, longitude, radius)`.")?
             }
+            ErrorKind::GeoBoundingBox => {
+                writeln!(f, "The `_geoBoundingBox` filter expects two pair of arguments: `_geoBoundingBox((latitude, longitude), (latitude, longitude))`.")?
+            }
             ErrorKind::ReservedGeo(name) => {
                 writeln!(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` coordinates.", name.escape_debug())?
             }
-            ErrorKind::MisusedGeo => {
+            ErrorKind::MisusedGeoRadius => {
                 writeln!(f, "The `_geoRadius` filter is an operation and can't be used as a value.")?
             }
+            ErrorKind::MisusedGeoBoundingBox => {
+                writeln!(f, "The `_geoBoundingBox` filter is an operation and can't be used as a value.")?
+            }
             ErrorKind::ReservedKeyword(word) => {
                 writeln!(f, "`{word}` is a reserved keyword and thus cannot be used as a field name unless it is put inside quotes. Use \"{word}\" or \'{word}\' instead.")?
             }

filter-parser/src/lib.rs

@@ -45,6 +45,7 @@ mod error;
 mod value;
 
 use std::fmt::Debug;
+use std::str::FromStr;
 
 pub use condition::{parse_condition, parse_to, Condition};
 use condition::{parse_exists, parse_not_exists};
@@ -71,7 +72,7 @@ const MAX_FILTER_DEPTH: usize = 200;
 #[derive(Debug, Clone, Eq)]
 pub struct Token<'a> {
     /// The token in the original input, it should be used when possible.
-    span: Span<'a>,
+    pub span: Span<'a>,
     /// If you need to modify the original input you can use the `value` field
     /// to store your modified input.
     value: Option<String>,
@@ -101,7 +102,7 @@ impl<'a> Token<'a> {
     }
 
     pub fn parse_finite_float(&self) -> Result<f64, Error> {
-        let value: f64 = self.span.parse().map_err(|e| self.as_external_error(e))?;
+        let value: f64 = self.value().parse().map_err(|e| self.as_external_error(e))?;
         if value.is_finite() {
             Ok(value)
         } else {
@@ -131,7 +132,7 @@ pub enum FilterCondition<'a> {
     Or(Vec<Self>),
     And(Vec<Self>),
     GeoLowerThan { point: [Token<'a>; 2], radius: Token<'a> },
-    GeoBoundingBox { top_left_point: [Token<'a>; 2], bottom_right_point: [Token<'a>; 2]},
+    GeoBoundingBox { top_left_point: [Token<'a>; 2], bottom_right_point: [Token<'a>; 2] },
 }
 
 impl<'a> FilterCondition<'a> {
@@ -312,12 +313,12 @@ fn parse_geo_radius(input: Span) -> IResult<FilterCondition> {
         // if we were able to parse `_geoRadius` and can't parse the rest of the input we return a failure
         cut(delimited(char('('), separated_list1(tag(","), ws(recognize_float)), char(')'))),
     )(input)
-    .map_err(|e| e.map(|_| Error::new_from_kind(input, ErrorKind::Geo)));
+    .map_err(|e| e.map(|_| Error::new_from_kind(input, ErrorKind::GeoRadius)));
 
     let (input, args) = parsed?;
 
     if args.len() != 3 {
-        return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::Geo)));
+        return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::GeoRadius)));
     }
 
     let res = FilterCondition::GeoLowerThan {
@@ -334,38 +335,27 @@ fn parse_geo_bounding_box(input: Span) -> IResult<FilterCondition> {
     let parsed = preceded(
         tuple((multispace0, word_exact("_geoBoundingBox"))),
         // if we were able to parse `_geoBoundingBox` and can't parse the rest of the input we return a failure
-        cut(
+        cut(delimited(
-            delimited(
+            char('('),
-                char('('),
+            separated_list1(
-                separated_list1(
+                tag(","),
-                    tag(","),
+                ws(delimited(char('('), separated_list1(tag(","), ws(recognize_float)), char(')'))),
-                    ws(
+            ),
-                        delimited(
+            char(')'),
-                            char('('),
+        )),
-                            separated_list1(
-                                tag(","),
-                                ws(recognize_float)
-                            ),
-                            char(')')
-                        )
-                    )
-                ),
-                char(')')
-            )
-        ),
     )(input)
-    .map_err(|e| e.map(|_| Error::new_from_kind(input, ErrorKind::Geo)));
+    .map_err(|e| e.map(|_| Error::new_from_kind(input, ErrorKind::GeoBoundingBox)));
 
     let (input, args) = parsed?;
 
-    if args.len() != 2 {
+    if args.len() != 2 || args[0].len() != 2 || args[1].len() != 2 {
-        return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::Geo)));
+        return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::GeoBoundingBox)));
     }
 
     //TODO: Check sub array length
     let res = FilterCondition::GeoBoundingBox {
         top_left_point: [args[0][0].into(), args[0][1].into()],
-        bottom_right_point: [args[1][0].into(), args[1][1].into()]
+        bottom_right_point: [args[1][0].into(), args[1][1].into()],
     };
     Ok((input, res))
 }
@@ -762,7 +752,14 @@ impl<'a> std::fmt::Display for FilterCondition<'a> {
                 write!(f, "_geoRadius({}, {}, {})", point[0], point[1], radius)
             }
             FilterCondition::GeoBoundingBox { top_left_point, bottom_right_point } => {
-                write!(f, "_geoBoundingBox(({}, {}), ({}, {}))", top_left_point[0], top_left_point[1], bottom_right_point[0], bottom_right_point[1])
+                write!(
+                    f,
+                    "_geoBoundingBox(({}, {}), ({}, {}))",
+                    top_left_point[0],
+                    top_left_point[1],
+                    bottom_right_point[0],
+                    bottom_right_point[1]
+                )
             }
         }
     }

filter-parser/src/value.rs

@@ -6,7 +6,7 @@ use nom::sequence::{delimited, terminated};
 use nom::{InputIter, InputLength, InputTake, Slice};
 
 use crate::error::{ExpectedValueKind, NomErrorExt};
-use crate::{parse_geo_point, parse_geo_radius, Error, ErrorKind, IResult, Span, Token};
+use crate::{parse_geo_point, parse_geo_radius, parse_geo_bounding_box, Error, ErrorKind, IResult, Span, Token};
 
 /// This function goes through all characters in the [Span] if it finds any escaped character (`\`).
 /// It generates a new string with all `\` removed from the [Span].
@@ -91,11 +91,21 @@ pub fn parse_value(input: Span) -> IResult<Token> {
         }
     }
     match parse_geo_radius(input) {
-        Ok(_) => return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::MisusedGeo))),
+        Ok(_) => return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::MisusedGeoRadius))),
         // if we encountered a failure it means the user badly wrote a _geoRadius filter.
         // But instead of showing him how to fix his syntax we are going to tell him he should not use this filter as a value.
         Err(e) if e.is_failure() => {
-            return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::MisusedGeo)))
+            return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::MisusedGeoRadius)))
+        }
+        _ => (),
+    }
+
+    match parse_geo_bounding_box(input) {
+        Ok(_) => return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::MisusedGeoBoundingBox))),
+        // if we encountered a failure it means the user badly wrote a _geoBoundingBox filter.
+        // But instead of showing him how to fix his syntax we are going to tell him he should not use this filter as a value.
+        Err(e) if e.is_failure() => {
+            return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::MisusedGeoBoundingBox)))
         }
         _ => (),
     }
@@ -155,7 +165,7 @@ fn is_syntax_component(c: char) -> bool {
 }
 
 fn is_keyword(s: &str) -> bool {
-    matches!(s, "AND" | "OR" | "IN" | "NOT" | "TO" | "EXISTS" | "_geoRadius")
+    matches!(s, "AND" | "OR" | "IN" | "NOT" | "TO" | "EXISTS" | "_geoRadius" | "_geoBoundingBox")
 }
 
 #[cfg(test)]

milli/src/asc_desc.rs

@@ -55,6 +55,9 @@ impl From<AscDescError> for CriterionError {
             AscDescError::ReservedKeyword { name } if name.starts_with("_geoRadius") => {
                 CriterionError::ReservedNameForFilter { name: "_geoRadius".to_string() }
             }
+            AscDescError::ReservedKeyword { name } if name.starts_with("_geoBoundingBox") => {
+                CriterionError::ReservedNameForFilter { name: "_geoBoundingBox".to_string() }
+            }
             AscDescError::ReservedKeyword { name } => CriterionError::ReservedName { name },
         }
     }

milli/src/criterion.rs

@@ -159,6 +159,8 @@ mod tests {
             ("_geoPoint(42, 75):asc", ReservedNameForSort { name: S("_geoPoint") }),
             ("_geoRadius:asc", ReservedNameForFilter { name: S("_geoRadius") }),
             ("_geoRadius(42, 75, 59):asc", ReservedNameForFilter { name: S("_geoRadius") }),
+            ("_geoBoundingBox:asc", ReservedNameForFilter { name: S("_geoBoundingBox") }),
+            ("_geoBoundinxBox((42, 75), (75, 59)):asc", ReservedNameForFilter { name: S("_geoBoundingBox") }),
         ];
 
         for (input, expected) in invalid_criteria {

milli/src/search/facet/filter.rs

@@ -385,6 +385,114 @@ impl<'a> Filter<'a> {
                     }))?
                 }
             }
+            FilterCondition::GeoBoundingBox { top_left_point, bottom_right_point } => {
+                if filterable_fields.contains("_geo") {
+                    let top_left: [f64; 2] = [
+                        top_left_point[0].parse_finite_float()?,
+                        top_left_point[1].parse_finite_float()?,
+                    ];
+                    let bottom_right: [f64; 2] = [
+                        bottom_right_point[0].parse_finite_float()?,
+                        bottom_right_point[1].parse_finite_float()?,
+                    ];
+                    if !(-90.0..=90.0).contains(&top_left[0]) {
+                        return Err(top_left_point[0]
+                            .as_external_error(FilterError::BadGeoLat(top_left[0])))?;
+                    }
+                    if !(-180.0..=180.0).contains(&top_left[1]) {
+                        return Err(top_left_point[1]
+                            .as_external_error(FilterError::BadGeoLng(top_left[1])))?;
+                    }
+                    if !(-90.0..=90.0).contains(&bottom_right[0]) {
+                        return Err(bottom_right_point[0]
+                            .as_external_error(FilterError::BadGeoLat(bottom_right[0])))?;
+                    }
+                    if !(-180.0..=180.0).contains(&bottom_right[1]) {
+                        return Err(bottom_right_point[1]
+                            .as_external_error(FilterError::BadGeoLng(bottom_right[1])))?;
+                    }
+
+                    let geo_lat_token =
+                        Token::new(top_left_point[0].span, Some("_geo.lat".to_string()));
+
+                    let condition_lat = FilterCondition::Condition {
+                        fid: geo_lat_token,
+                        op: Condition::Between {
+                            from: bottom_right_point[0].clone(),
+                            to: top_left_point[0].clone(),
+                        },
+                    };
+
+                    let selected_lat = Filter { condition: condition_lat }.inner_evaluate(
+                        rtxn,
+                        index,
+                        filterable_fields,
+                    )?;
+
+                    let geo_lng_token =
+                        Token::new(top_left_point[1].span, Some("_geo.lng".to_string()));
+                    let min_lng_token =
+                        Token::new(top_left_point[1].span, Some("-180.0".to_string()));
+                    let max_lng_token =
+                        Token::new(top_left_point[1].span, Some("180.0".to_string()));
+
+                    let selected_lng = if top_left[1] > bottom_right[1] {
+                        dbg!("test");
+
+                        let condition_left = FilterCondition::Condition {
+                            fid: geo_lng_token.clone(),
+                            op: Condition::Between {
+                                from: dbg!(top_left_point[1].clone()),
+                                to: max_lng_token,
+                            },
+                        };
+                        let left = Filter { condition: condition_left }.inner_evaluate(
+                            rtxn,
+                            index,
+                            filterable_fields,
+                        )?;
+
+                        let condition_right = FilterCondition::Condition {
+                            fid: geo_lng_token,
+                            op: Condition::Between {
+                                from: dbg!(min_lng_token),
+                                to: dbg!(bottom_right_point[1].clone()),
+                            },
+                        };
+                        let right = Filter { condition: condition_right }.inner_evaluate(
+                            rtxn,
+                            index,
+                            filterable_fields,
+                        )?;
+
+                        dbg!(&left);
+                        dbg!(&right);
+                        dbg!(left | right)
+                    } else {
+                        let condition_lng = FilterCondition::Condition {
+                            fid: geo_lng_token,
+                            op: Condition::Between {
+                                from: top_left_point[1].clone(),
+                                to: bottom_right_point[1].clone(),
+                            },
+                        };
+                        Filter { condition: condition_lng }.inner_evaluate(
+                            rtxn,
+                            index,
+                            filterable_fields,
+                        )?
+                    };
+
+                    dbg!(&selected_lng);
+
+                    Ok(selected_lat & selected_lng)
+                } else {
+                    Err(top_left_point[0].as_external_error(FilterError::AttributeNotFilterable {
+                        attribute: "_geo",
+                        filterable_fields: filterable_fields.clone(),
+                    }))?
+                }
+            }
         }
     }
 }