From dc64170a69e0be95b2c9e5bb96e1a5be0b58ec06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Lecrenier?= Date: Tue, 14 Jun 2022 16:42:09 +0200 Subject: [PATCH] =?UTF-8?q?Improve=20syntax=20of=20EXISTS=20filter,=20allo?= =?UTF-8?q?w=20=E2=80=9Cvalue=20NOT=20EXISTS=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- filter-parser/src/condition.rs | 22 ++++++++++------ filter-parser/src/error.rs | 4 +-- filter-parser/src/lib.rs | 45 +++++++++++++++++++------------- milli/src/search/facet/filter.rs | 6 ++--- 4 files changed, 46 insertions(+), 31 deletions(-) diff --git a/filter-parser/src/condition.rs b/filter-parser/src/condition.rs index 0ece99a0d..b57d68b75 100644 --- a/filter-parser/src/condition.rs +++ b/filter-parser/src/condition.rs @@ -19,8 +19,8 @@ pub enum Condition<'a> { GreaterThanOrEqual(Token<'a>), Equal(Token<'a>), NotEqual(Token<'a>), - Exist, - NotExist, + Exists, + NotExists, LowerThan(Token<'a>), LowerThanOrEqual(Token<'a>), Between { from: Token<'a>, to: Token<'a> }, @@ -35,8 +35,8 @@ impl<'a> Condition<'a> { GreaterThanOrEqual(n) => (LowerThan(n), None), Equal(s) => (NotEqual(s), None), NotEqual(s) => (Equal(s), None), - Exist => (NotExist, None), - NotExist => (Exist, None), + Exists => (NotExists, None), + NotExists => (Exists, None), LowerThan(n) => (GreaterThanOrEqual(n), None), LowerThanOrEqual(n) => (GreaterThan(n), None), Between { from, to } => (LowerThan(from), Some(GreaterThan(to))), @@ -62,11 +62,17 @@ pub fn parse_condition(input: Span) -> IResult { Ok((input, condition)) } -/// exist = value EXIST -pub fn parse_exist(input: Span) -> IResult { - let (input, key) = terminated(parse_value, tag("EXIST"))(input)?; +/// exist = value NOT EXISTS +pub fn parse_exists(input: Span) -> IResult { + let (input, key) = terminated(parse_value, tag("EXISTS"))(input)?; - Ok((input, FilterCondition::Condition { fid: key.into(), op: Exist })) + Ok((input, FilterCondition::Condition { fid: key.into(), op: Exists })) +} +/// exist = value NOT EXISTS +pub fn parse_not_exists(input: Span) -> IResult { + let (input, key) = terminated(parse_value, tag("NOT EXISTS"))(input)?; + + Ok((input, FilterCondition::Condition { fid: key.into(), op: NotExists })) } /// to = value value TO value diff --git a/filter-parser/src/error.rs b/filter-parser/src/error.rs index 8136732c8..a3720f7bf 100644 --- a/filter-parser/src/error.rs +++ b/filter-parser/src/error.rs @@ -128,10 +128,10 @@ impl<'a> Display for Error<'a> { writeln!(f, "Was expecting a value but instead got `{}`.", escaped_input)? } ErrorKind::InvalidPrimary if input.trim().is_empty() => { - writeln!(f, "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXIST`, or `_geoRadius` but instead got nothing.")? + writeln!(f, "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXISTS`, `NOT EXISTS`, or `_geoRadius` but instead got nothing.")? } ErrorKind::InvalidPrimary => { - writeln!(f, "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXIST`, or `_geoRadius` at `{}`.", escaped_input)? + writeln!(f, "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXISTS`, `NOT EXISTS`, or `_geoRadius` at `{}`.", escaped_input)? } ErrorKind::ExpectedEof => { writeln!(f, "Found unexpected characters at the end of the filter: `{}`. You probably forgot an `OR` or an `AND` rule.", escaped_input)? diff --git a/filter-parser/src/lib.rs b/filter-parser/src/lib.rs index ee4edc122..69215798c 100644 --- a/filter-parser/src/lib.rs +++ b/filter-parser/src/lib.rs @@ -3,23 +3,24 @@ //! ```text //! filter = expression ~ EOF //! expression = or -//! or = and (~ "OR" ~ and) -//! and = not (~ "AND" not)* -//! not = ("NOT" ~ not) | primary -//! primary = (WS* ~ "(" expression ")" ~ WS*) | geoRadius | condition | exist | to +//! or = and ("OR" and) +//! and = not ("AND" not)* +//! not = ("NOT" not) | primary +//! primary = (WS* "(" expression ")" WS*) | geoRadius | condition | exists | not_exists | to //! condition = value ("==" | ">" ...) value -//! exist = value EXIST +//! exists = value EXISTS +//! not_exists = value NOT EXISTS //! to = value value TO value -//! value = WS* ~ ( word | singleQuoted | doubleQuoted) ~ WS* +//! value = WS* ( word | singleQuoted | doubleQuoted) ~ WS* //! singleQuoted = "'" .* all but quotes "'" //! doubleQuoted = "\"" .* all but double quotes "\"" //! word = (alphanumeric | _ | - | .)+ -//! geoRadius = WS* ~ "_geoRadius(" ~ WS* ~ float ~ WS* ~ "," ~ WS* ~ float ~ WS* ~ "," float ~ WS* ~ ")" +//! geoRadius = WS* ~ "_geoRadius(" WS* float WS* "," WS* float WS* "," float WS* ")" //! ``` //! //! Other BNF grammar used to handle some specific errors: //! ```text -//! geoPoint = WS* ~ "_geoPoint(" ~ (float ~ ",")* ~ ")" +//! geoPoint = WS* "_geoPoint(" (float ",")* ")" //! ``` //! //! Specific errors: @@ -43,8 +44,8 @@ mod value; use std::fmt::Debug; use std::str::FromStr; -use condition::parse_exist; pub use condition::{parse_condition, parse_to, Condition}; +use condition::{parse_exists, parse_not_exists}; use error::{cut_with_err, NomErrorExt}; pub use error::{Error, ErrorKind}; use nom::branch::alt; @@ -250,7 +251,8 @@ fn parse_primary(input: Span) -> IResult { ), parse_geo_radius, parse_condition, - parse_exist, + parse_exists, + parse_not_exists, parse_to, // the next lines are only for error handling and are written at the end to have the less possible performance impact parse_geo_point, @@ -424,17 +426,24 @@ pub mod tests { }, ), ( - "subscribers EXIST", + "subscribers EXISTS", Fc::Condition { fid: rtok("", "subscribers"), - op: Condition::Exist, + op: Condition::Exists, }, ), ( - "NOT subscribers EXIST", + "NOT subscribers EXISTS", Fc::Condition { fid: rtok("NOT ", "subscribers"), - op: Condition::NotExist, + op: Condition::NotExists, + }, + ), + ( + "subscribers NOT EXISTS", + Fc::Condition { + fid: rtok("", "subscribers"), + op: Condition::NotExists, }, ), ( @@ -594,10 +603,10 @@ pub mod tests { ("channel = ", "Was expecting a value but instead got nothing."), ("channel = 🐻", "Was expecting a value but instead got `🐻`."), ("channel = 🐻 AND followers < 100", "Was expecting a value but instead got `🐻`."), - ("OR", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXIST`, or `_geoRadius` at `OR`."), - ("AND", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXIST`, or `_geoRadius` at `AND`."), - ("channel Ponce", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXIST`, or `_geoRadius` at `channel Ponce`."), - ("channel = Ponce OR", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXIST`, or `_geoRadius` but instead got nothing."), + ("OR", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXISTS`, `NOT EXISTS`, or `_geoRadius` at `OR`."), + ("AND", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXISTS`, `NOT EXISTS`, or `_geoRadius` at `AND`."), + ("channel Ponce", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXISTS`, `NOT EXISTS`, or `_geoRadius` at `channel Ponce`."), + ("channel = Ponce OR", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXISTS`, `NOT EXISTS`, or `_geoRadius` but instead got nothing."), ("_geoRadius", "The `_geoRadius` filter expects three arguments: `_geoRadius(latitude, longitude, radius)`."), ("_geoRadius = 12", "The `_geoRadius` filter expects three arguments: `_geoRadius(latitude, longitude, radius)`."), ("_geoPoint(12, 13, 14)", "`_geoPoint` 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."), diff --git a/milli/src/search/facet/filter.rs b/milli/src/search/facet/filter.rs index a5c13ec2a..7f3b928dd 100644 --- a/milli/src/search/facet/filter.rs +++ b/milli/src/search/facet/filter.rs @@ -280,11 +280,11 @@ impl<'a> Filter<'a> { Condition::LowerThan(val) => (Included(f64::MIN), Excluded(val.parse()?)), Condition::LowerThanOrEqual(val) => (Included(f64::MIN), Included(val.parse()?)), Condition::Between { from, to } => (Included(from.parse()?), Included(to.parse()?)), - Condition::Exist => { + Condition::Exists => { let exist = index.exists_faceted_documents_ids(rtxn, field_id)?; return Ok(exist); } - Condition::NotExist => { + Condition::NotExists => { let all_ids = index.documents_ids(rtxn)?; let exist = Self::evaluate_operator( @@ -293,7 +293,7 @@ impl<'a> Filter<'a> { numbers_db, strings_db, field_id, - &Condition::Exist, + &Condition::Exists, )?; let notexist = all_ids - exist;