Improve syntax of EXISTS filter, allow “value NOT EXISTS”

This commit is contained in:
Loïc Lecrenier 2022-06-14 16:42:09 +02:00
parent 72452f0cb2
commit dc64170a69
4 changed files with 46 additions and 31 deletions

View File

@ -19,8 +19,8 @@ pub enum Condition<'a> {
GreaterThanOrEqual(Token<'a>), GreaterThanOrEqual(Token<'a>),
Equal(Token<'a>), Equal(Token<'a>),
NotEqual(Token<'a>), NotEqual(Token<'a>),
Exist, Exists,
NotExist, NotExists,
LowerThan(Token<'a>), LowerThan(Token<'a>),
LowerThanOrEqual(Token<'a>), LowerThanOrEqual(Token<'a>),
Between { from: Token<'a>, to: Token<'a> }, Between { from: Token<'a>, to: Token<'a> },
@ -35,8 +35,8 @@ impl<'a> Condition<'a> {
GreaterThanOrEqual(n) => (LowerThan(n), None), GreaterThanOrEqual(n) => (LowerThan(n), None),
Equal(s) => (NotEqual(s), None), Equal(s) => (NotEqual(s), None),
NotEqual(s) => (Equal(s), None), NotEqual(s) => (Equal(s), None),
Exist => (NotExist, None), Exists => (NotExists, None),
NotExist => (Exist, None), NotExists => (Exists, None),
LowerThan(n) => (GreaterThanOrEqual(n), None), LowerThan(n) => (GreaterThanOrEqual(n), None),
LowerThanOrEqual(n) => (GreaterThan(n), None), LowerThanOrEqual(n) => (GreaterThan(n), None),
Between { from, to } => (LowerThan(from), Some(GreaterThan(to))), Between { from, to } => (LowerThan(from), Some(GreaterThan(to))),
@ -62,11 +62,17 @@ pub fn parse_condition(input: Span) -> IResult<FilterCondition> {
Ok((input, condition)) Ok((input, condition))
} }
/// exist = value EXIST /// exist = value NOT EXISTS
pub fn parse_exist(input: Span) -> IResult<FilterCondition> { pub fn parse_exists(input: Span) -> IResult<FilterCondition> {
let (input, key) = terminated(parse_value, tag("EXIST"))(input)?; 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<FilterCondition> {
let (input, key) = terminated(parse_value, tag("NOT EXISTS"))(input)?;
Ok((input, FilterCondition::Condition { fid: key.into(), op: NotExists }))
} }
/// to = value value TO value /// to = value value TO value

View File

@ -128,10 +128,10 @@ impl<'a> Display for Error<'a> {
writeln!(f, "Was expecting a value but instead got `{}`.", escaped_input)? writeln!(f, "Was expecting a value but instead got `{}`.", escaped_input)?
} }
ErrorKind::InvalidPrimary if input.trim().is_empty() => { 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 => { 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 => { ErrorKind::ExpectedEof => {
writeln!(f, "Found unexpected characters at the end of the filter: `{}`. You probably forgot an `OR` or an `AND` rule.", escaped_input)? writeln!(f, "Found unexpected characters at the end of the filter: `{}`. You probably forgot an `OR` or an `AND` rule.", escaped_input)?

View File

@ -3,23 +3,24 @@
//! ```text //! ```text
//! filter = expression ~ EOF //! filter = expression ~ EOF
//! expression = or //! expression = or
//! or = and (~ "OR" ~ and) //! or = and ("OR" and)
//! and = not (~ "AND" not)* //! and = not ("AND" not)*
//! not = ("NOT" ~ not) | primary //! not = ("NOT" not) | primary
//! primary = (WS* ~ "(" expression ")" ~ WS*) | geoRadius | condition | exist | to //! primary = (WS* "(" expression ")" WS*) | geoRadius | condition | exists | not_exists | to
//! condition = value ("==" | ">" ...) value //! condition = value ("==" | ">" ...) value
//! exist = value EXIST //! exists = value EXISTS
//! not_exists = value NOT EXISTS
//! to = value value TO value //! to = value value TO value
//! value = WS* ~ ( word | singleQuoted | doubleQuoted) ~ WS* //! value = WS* ( word | singleQuoted | doubleQuoted) ~ WS*
//! singleQuoted = "'" .* all but quotes "'" //! singleQuoted = "'" .* all but quotes "'"
//! doubleQuoted = "\"" .* all but double quotes "\"" //! doubleQuoted = "\"" .* all but double quotes "\""
//! word = (alphanumeric | _ | - | .)+ //! 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: //! Other BNF grammar used to handle some specific errors:
//! ```text //! ```text
//! geoPoint = WS* ~ "_geoPoint(" ~ (float ~ ",")* ~ ")" //! geoPoint = WS* "_geoPoint(" (float ",")* ")"
//! ``` //! ```
//! //!
//! Specific errors: //! Specific errors:
@ -43,8 +44,8 @@ mod value;
use std::fmt::Debug; use std::fmt::Debug;
use std::str::FromStr; use std::str::FromStr;
use condition::parse_exist;
pub use condition::{parse_condition, parse_to, Condition}; pub use condition::{parse_condition, parse_to, Condition};
use condition::{parse_exists, parse_not_exists};
use error::{cut_with_err, NomErrorExt}; use error::{cut_with_err, NomErrorExt};
pub use error::{Error, ErrorKind}; pub use error::{Error, ErrorKind};
use nom::branch::alt; use nom::branch::alt;
@ -250,7 +251,8 @@ fn parse_primary(input: Span) -> IResult<FilterCondition> {
), ),
parse_geo_radius, parse_geo_radius,
parse_condition, parse_condition,
parse_exist, parse_exists,
parse_not_exists,
parse_to, parse_to,
// the next lines are only for error handling and are written at the end to have the less possible performance impact // the next lines are only for error handling and are written at the end to have the less possible performance impact
parse_geo_point, parse_geo_point,
@ -424,17 +426,24 @@ pub mod tests {
}, },
), ),
( (
"subscribers EXIST", "subscribers EXISTS",
Fc::Condition { Fc::Condition {
fid: rtok("", "subscribers"), fid: rtok("", "subscribers"),
op: Condition::Exist, op: Condition::Exists,
}, },
), ),
( (
"NOT subscribers EXIST", "NOT subscribers EXISTS",
Fc::Condition { Fc::Condition {
fid: rtok("NOT ", "subscribers"), 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 nothing."),
("channel = 🐻", "Was expecting a value but instead got `🐻`."), ("channel = 🐻", "Was expecting a value but instead got `🐻`."),
("channel = 🐻 AND followers < 100", "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`."), ("OR", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXISTS`, `NOT EXISTS`, or `_geoRadius` at `OR`."),
("AND", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXIST`, or `_geoRadius` at `AND`."), ("AND", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXISTS`, `NOT EXISTS`, or `_geoRadius` at `AND`."),
("channel Ponce", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXIST`, or `_geoRadius` at `channel Ponce`."), ("channel Ponce", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXISTS`, `NOT EXISTS`, or `_geoRadius` at `channel Ponce`."),
("channel = Ponce OR", "Was expecting an operation `=`, `!=`, `>=`, `>`, `<=`, `<`, `TO`, `EXIST`, or `_geoRadius` but instead got nothing."), ("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", "The `_geoRadius` filter expects three arguments: `_geoRadius(latitude, longitude, radius)`."),
("_geoRadius = 12", "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."), ("_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."),

View File

@ -280,11 +280,11 @@ impl<'a> Filter<'a> {
Condition::LowerThan(val) => (Included(f64::MIN), Excluded(val.parse()?)), Condition::LowerThan(val) => (Included(f64::MIN), Excluded(val.parse()?)),
Condition::LowerThanOrEqual(val) => (Included(f64::MIN), Included(val.parse()?)), Condition::LowerThanOrEqual(val) => (Included(f64::MIN), Included(val.parse()?)),
Condition::Between { from, to } => (Included(from.parse()?), Included(to.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)?; let exist = index.exists_faceted_documents_ids(rtxn, field_id)?;
return Ok(exist); return Ok(exist);
} }
Condition::NotExist => { Condition::NotExists => {
let all_ids = index.documents_ids(rtxn)?; let all_ids = index.documents_ids(rtxn)?;
let exist = Self::evaluate_operator( let exist = Self::evaluate_operator(
@ -293,7 +293,7 @@ impl<'a> Filter<'a> {
numbers_db, numbers_db,
strings_db, strings_db,
field_id, field_id,
&Condition::Exist, &Condition::Exists,
)?; )?;
let notexist = all_ids - exist; let notexist = all_ids - exist;