diff --git a/filter-parser/src/error.rs b/filter-parser/src/error.rs index ea95caba7..aaf1a2e36 100644 --- a/filter-parser/src/error.rs +++ b/filter-parser/src/error.rs @@ -57,8 +57,10 @@ pub enum ExpectedValueKind { #[derive(Debug)] pub enum ErrorKind<'a> { ReservedGeo(&'a str), - Geo, - MisusedGeo, + GeoRadius, + 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.")? } diff --git a/filter-parser/src/lib.rs b/filter-parser/src/lib.rs index 65c4c56f8..89e80a267 100644 --- a/filter-parser/src/lib.rs +++ b/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, @@ -101,7 +102,7 @@ impl<'a> Token<'a> { } pub fn parse_finite_float(&self) -> Result { - 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), And(Vec), 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 { // 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 { 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( - delimited( - char('('), - separated_list1( - tag(","), - ws( - delimited( - char('('), - separated_list1( - tag(","), - ws(recognize_float) - ), - char(')') - ) - ) - ), - char(')') - ) - ), + cut(delimited( + char('('), + separated_list1( + tag(","), + ws(delimited(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 { - return Err(nom::Err::Failure(Error::new_from_kind(input, ErrorKind::Geo))); + if args.len() != 2 || args[0].len() != 2 || args[1].len() != 2 { + 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] + ) } } } diff --git a/filter-parser/src/value.rs b/filter-parser/src/value.rs index 73ef61480..abdc10439 100644 --- a/filter-parser/src/value.rs +++ b/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 { } } 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)] diff --git a/milli/src/asc_desc.rs b/milli/src/asc_desc.rs index 21065da36..826290c8a 100644 --- a/milli/src/asc_desc.rs +++ b/milli/src/asc_desc.rs @@ -55,6 +55,9 @@ impl From 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 }, } } diff --git a/milli/src/criterion.rs b/milli/src/criterion.rs index c02cd2525..4544a97ac 100644 --- a/milli/src/criterion.rs +++ b/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 { diff --git a/milli/src/search/facet/filter.rs b/milli/src/search/facet/filter.rs index 23cbb280c..b44db29e4 100644 --- a/milli/src/search/facet/filter.rs +++ b/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(), + }))? + } + } } } }