From 501e3816a8e6b4402dd9eea2666f18427ddba5c9 Mon Sep 17 00:00:00 2001 From: Tamo Date: Wed, 17 Sep 2025 16:26:24 +0200 Subject: [PATCH] review the filters errors --- crates/filter-parser/src/error.rs | 6 +++--- crates/filter-parser/src/lib.rs | 9 ++++++--- crates/meilisearch-types/src/error.rs | 1 + crates/milli/src/search/facet/filter.rs | 7 ++----- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/crates/filter-parser/src/error.rs b/crates/filter-parser/src/error.rs index 88bf90be2..d1434e20d 100644 --- a/crates/filter-parser/src/error.rs +++ b/crates/filter-parser/src/error.rs @@ -78,7 +78,7 @@ pub enum ErrorKind<'a> { GeoRadiusArgumentCount(usize), GeoBoundingBox, GeoPolygon, - GeoPolygonTooFewPoints, + GeoPolygonNotEnoughPoints(usize), GeoCoordinatesNotPair(usize), MisusedGeoRadius, MisusedGeoBoundingBox, @@ -213,8 +213,8 @@ impl Display for Error<'_> { ErrorKind::GeoPolygon => { writeln!(f, "The `_geoPolygon` filter doesn't match the expected format: `_geoPolygon([latitude, longitude], [latitude, longitude])`.")? } - ErrorKind::GeoPolygonTooFewPoints => { - writeln!(f, "The `_geoPolygon` filter expects at least 2 points")?; + ErrorKind::GeoPolygonNotEnoughPoints(n) => { + writeln!(f, "The `_geoPolygon` filter expects at least 3 points but only {n} were specified")?; } ErrorKind::GeoCoordinatesNotPair(number) => { writeln!(f, "Was expecting 2 coordinates but instead found {number}.")? diff --git a/crates/filter-parser/src/lib.rs b/crates/filter-parser/src/lib.rs index e9b2b090b..cbe611ff7 100644 --- a/crates/filter-parser/src/lib.rs +++ b/crates/filter-parser/src/lib.rs @@ -465,7 +465,7 @@ fn parse_geo_bounding_box(input: Span) -> IResult { /// geoPolygon = "_geoPolygon([[" WS* float WS* "," WS* float WS* "],+])" /// If we parse `_geoPolygon` we MUST parse the rest of the expression. fn parse_geo_polygon(input: Span) -> IResult { - // we want to allow space BEFORE the _geoBoundingBox but not after + // we want to allow space BEFORE the _geoPolygon but not after let (input, _) = tuple((multispace0, word_exact("_geoPolygon")))(input)?; @@ -481,9 +481,12 @@ fn parse_geo_polygon(input: Span) -> IResult { )(input) .map_cut(ErrorKind::GeoPolygon)?; - if args.len() < 2 { + if args.len() < 3 { let context = args.last().and_then(|a| a.last()).unwrap_or(&input); - return Err(Error::failure_from_kind(*context, ErrorKind::GeoPolygonTooFewPoints)); + return Err(Error::failure_from_kind( + *context, + ErrorKind::GeoPolygonNotEnoughPoints(args.len()), + )); } if let Some(offending) = args.iter().find(|a| a.len() != 2) { diff --git a/crates/meilisearch-types/src/error.rs b/crates/meilisearch-types/src/error.rs index d45580274..902454b0b 100644 --- a/crates/meilisearch-types/src/error.rs +++ b/crates/meilisearch-types/src/error.rs @@ -539,6 +539,7 @@ impl ErrorCode for milli::Error { | cellulite::Error::CannotConvertLineToCell(_, _, _) => Code::Internal, cellulite::Error::InvalidGeoJson(_) => Code::InvalidDocumentGeojsonField, }, + UserError::MalformedGeojson(_) => Code::InvalidDocumentGeojsonField, } } } diff --git a/crates/milli/src/search/facet/filter.rs b/crates/milli/src/search/facet/filter.rs index a234ecb16..c8049ab99 100644 --- a/crates/milli/src/search/facet/filter.rs +++ b/crates/milli/src/search/facet/filter.rs @@ -699,12 +699,9 @@ impl<'a> Filter<'a> { if index.is_geojson_filtering_enabled(rtxn)? { let point = geo_types::Point::new(base_point[1], base_point[0]); - let result = index - .cellulite - .in_circle(rtxn, point, radius, resolution) - .map_err(InternalError::CelluliteError)?; + let result = index.cellulite.in_circle(rtxn, point, radius, resolution)?; - r2 = Some(RoaringBitmap::from_iter(result)); // TODO: Remove once we update roaring + r2 = Some(RoaringBitmap::from_iter(result)); // TODO: Remove once we update roaring in meilisearch } match (r1, r2) {