diff --git a/Cargo.toml b/Cargo.toml index aef509c44..a22037c94 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,7 @@ owning_ref = "0.4" stable_deref_trait = "1.0.0" rust-stemmers = "1.2" downcast-rs = { version="1.0" } -tantivy-query-grammar = { version="0.12", path="./query-grammar" } +tantivy-query-grammar = { version="0.13", path="./query-grammar" } bitpacking = {version="0.8", default-features = false, features=["bitpacker4x"]} census = "0.4" fnv = "1.0.6" diff --git a/appveyor.yml b/appveyor.yml index 7f9035e03..16e7055eb 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -18,5 +18,5 @@ install: build: false test_script: - - REM SET RUST_LOG=tantivy,test & cargo test --verbose --no-default-features --features mmap + - REM SET RUST_LOG=tantivy,test & cargo test --all --verbose --no-default-features --features mmap - REM SET RUST_BACKTRACE=1 & cargo build --examples diff --git a/query-grammar/Cargo.toml b/query-grammar/Cargo.toml index 46a746258..d490c1ee4 100644 --- a/query-grammar/Cargo.toml +++ b/query-grammar/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "tantivy-query-grammar" -version = "0.12.0" +version = "0.13.0" authors = ["Paul Masurel "] license = "MIT" categories = ["database-implementations", "data-structures"] diff --git a/query-grammar/src/query_grammar.rs b/query-grammar/src/query_grammar.rs index 5976f583b..e7620613b 100644 --- a/query-grammar/src/query_grammar.rs +++ b/query-grammar/src/query_grammar.rs @@ -154,17 +154,11 @@ fn negate(expr: UserInputAST) -> UserInputAST { expr.unary(Occur::MustNot) } -fn must(expr: UserInputAST) -> UserInputAST { - expr.unary(Occur::Must) -} - fn leaf<'a>() -> impl Parser<&'a str, Output = UserInputAST> { parser(|input| { - char('-') - .with(leaf()) - .map(negate) - .or(char('+').with(leaf()).map(must)) - .or(char('(').with(ast()).skip(char(')'))) + char('(') + .with(ast()) + .skip(char(')')) .or(char('*').map(|_| UserInputAST::from(UserInputLeaf::All))) .or(attempt( string("NOT").skip(spaces1()).with(leaf()).map(negate), @@ -176,6 +170,16 @@ fn leaf<'a>() -> impl Parser<&'a str, Output = UserInputAST> { }) } +fn occur_symbol<'a>() -> impl Parser<&'a str, Output = Occur> { + char('-') + .map(|_| Occur::MustNot) + .or(char('+').map(|_| Occur::Must)) +} + +fn occur_leaf<'a>() -> impl Parser<&'a str, Output = (Option, UserInputAST)> { + (optional(occur_symbol()), boosted_leaf()) +} + fn positive_float_number<'a>() -> impl Parser<&'a str, Output = f32> { (many1(digit()), optional((char('.'), many1(digit())))).map( |(int_part, decimal_part_opt): (String, Option<(char, String)>)| { @@ -239,21 +243,29 @@ fn aggregate_binary_expressions( } } -pub fn ast<'a>() -> impl Parser<&'a str, Output = UserInputAST> { - let operand_leaf = ( +fn operand_leaf<'a>() -> impl Parser<&'a str, Output = (BinaryOperand, UserInputAST)> { + ( binary_operand().skip(spaces()), boosted_leaf().skip(spaces()), - ); - let boolean_expr = (boosted_leaf().skip(spaces().silent()), many1(operand_leaf)) + ) +} + +pub fn ast<'a>() -> impl Parser<&'a str, Output = UserInputAST> { + let boolean_expr = (boosted_leaf().skip(spaces()), many1(operand_leaf())) .map(|(left, right)| aggregate_binary_expressions(left, right)); - let whitespace_separated_leaves = - many1(boosted_leaf().skip(spaces().silent())).map(|subqueries: Vec| { + let whitespace_separated_leaves = many1(occur_leaf().skip(spaces().silent())).map( + |subqueries: Vec<(Option, UserInputAST)>| { if subqueries.len() == 1 { - subqueries.into_iter().next().unwrap() + let (occur_opt, ast) = subqueries.into_iter().next().unwrap(); + match occur_opt.unwrap_or(Occur::Should) { + Occur::Must | Occur::Should => ast, + Occur::MustNot => UserInputAST::Clause(vec![(Some(Occur::MustNot), ast)]), + } } else { UserInputAST::Clause(subqueries.into_iter().collect()) } - }); + }, + ); let expr = attempt(boolean_expr).or(whitespace_separated_leaves); spaces().with(expr).skip(spaces()) } @@ -283,6 +295,12 @@ mod test { ); } + #[test] + fn test_occur_symbol() { + assert_eq!(super::occur_symbol().parse("-"), Ok((Occur::MustNot, ""))); + assert_eq!(super::occur_symbol().parse("+"), Ok((Occur::Must, ""))); + } + #[test] fn test_positive_float_number() { fn valid_parse(float_str: &str, expected_val: f32, expected_remaining: &str) { @@ -330,7 +348,7 @@ mod test { "Err(UnexpectedParse)" ); test_parse_query_to_ast_helper("NOTa", "\"NOTa\""); - test_parse_query_to_ast_helper("NOT a", "-(\"a\")"); + test_parse_query_to_ast_helper("NOT a", "(-\"a\")"); } #[test] @@ -338,16 +356,16 @@ mod test { assert!(parse_to_ast().parse("a^2^3").is_err()); assert!(parse_to_ast().parse("a^2^").is_err()); test_parse_query_to_ast_helper("a^3", "(\"a\")^3"); - test_parse_query_to_ast_helper("a^3 b^2", "((\"a\")^3 (\"b\")^2)"); + test_parse_query_to_ast_helper("a^3 b^2", "(*(\"a\")^3 *(\"b\")^2)"); test_parse_query_to_ast_helper("a^1", "\"a\""); } #[test] fn test_parse_query_to_ast_binary_op() { - test_parse_query_to_ast_helper("a AND b", "(+(\"a\") +(\"b\"))"); - test_parse_query_to_ast_helper("a OR b", "(?(\"a\") ?(\"b\"))"); - test_parse_query_to_ast_helper("a OR b AND c", "(?(\"a\") ?((+(\"b\") +(\"c\"))))"); - test_parse_query_to_ast_helper("a AND b AND c", "(+(\"a\") +(\"b\") +(\"c\"))"); + test_parse_query_to_ast_helper("a AND b", "(+\"a\" +\"b\")"); + test_parse_query_to_ast_helper("a OR b", "(?\"a\" ?\"b\")"); + test_parse_query_to_ast_helper("a OR b AND c", "(?\"a\" ?(+\"b\" +\"c\"))"); + test_parse_query_to_ast_helper("a AND b AND c", "(+\"a\" +\"b\" +\"c\")"); assert_eq!( format!("{:?}", parse_to_ast().parse("a OR b aaa")), "Err(UnexpectedParse)" @@ -385,6 +403,13 @@ mod test { test_parse_query_to_ast_helper("weight: <= 70.5", "weight:{\"*\" TO \"70.5\"]"); } + #[test] + fn test_occur_leaf() { + let ((occur, ast), _) = super::occur_leaf().parse("+abc").unwrap(); + assert_eq!(occur, Some(Occur::Must)); + assert_eq!(format!("{:?}", ast), "\"abc\""); + } + #[test] fn test_range_parser() { // testing the range() parser separately @@ -413,32 +438,67 @@ mod test { fn test_parse_query_to_triming_spaces() { test_parse_query_to_ast_helper(" abc", "\"abc\""); test_parse_query_to_ast_helper("abc ", "\"abc\""); - test_parse_query_to_ast_helper("( a OR abc)", "(?(\"a\") ?(\"abc\"))"); - test_parse_query_to_ast_helper("(a OR abc)", "(?(\"a\") ?(\"abc\"))"); - test_parse_query_to_ast_helper("(a OR abc)", "(?(\"a\") ?(\"abc\"))"); - test_parse_query_to_ast_helper("a OR abc ", "(?(\"a\") ?(\"abc\"))"); - test_parse_query_to_ast_helper("(a OR abc )", "(?(\"a\") ?(\"abc\"))"); - test_parse_query_to_ast_helper("(a OR abc) ", "(?(\"a\") ?(\"abc\"))"); + test_parse_query_to_ast_helper("( a OR abc)", "(?\"a\" ?\"abc\")"); + test_parse_query_to_ast_helper("(a OR abc)", "(?\"a\" ?\"abc\")"); + test_parse_query_to_ast_helper("(a OR abc)", "(?\"a\" ?\"abc\")"); + test_parse_query_to_ast_helper("a OR abc ", "(?\"a\" ?\"abc\")"); + test_parse_query_to_ast_helper("(a OR abc )", "(?\"a\" ?\"abc\")"); + test_parse_query_to_ast_helper("(a OR abc) ", "(?\"a\" ?\"abc\")"); } #[test] - fn test_parse_query_to_ast() { + fn test_parse_query_single_term() { test_parse_query_to_ast_helper("abc", "\"abc\""); - test_parse_query_to_ast_helper("a b", "(\"a\" \"b\")"); - test_parse_query_to_ast_helper("+(a b)", "+((\"a\" \"b\"))"); - test_parse_query_to_ast_helper("+d", "+(\"d\")"); - test_parse_query_to_ast_helper("+(a b) +d", "(+((\"a\" \"b\")) +(\"d\"))"); - test_parse_query_to_ast_helper("(+a +b) d", "((+(\"a\") +(\"b\")) \"d\")"); - test_parse_query_to_ast_helper("(+a)", "+(\"a\")"); - test_parse_query_to_ast_helper("(+a +b)", "(+(\"a\") +(\"b\"))"); + } + + #[test] + fn test_parse_query_default_clause() { + test_parse_query_to_ast_helper("a b", "(*\"a\" *\"b\")"); + } + + #[test] + fn test_parse_query_must_default_clause() { + test_parse_query_to_ast_helper("+(a b)", "(*\"a\" *\"b\")"); + } + + #[test] + fn test_parse_query_must_single_term() { + test_parse_query_to_ast_helper("+d", "\"d\""); + } + + #[test] + fn test_single_term_with_field() { test_parse_query_to_ast_helper("abc:toto", "abc:\"toto\""); + } + + #[test] + fn test_single_term_with_float() { test_parse_query_to_ast_helper("abc:1.1", "abc:\"1.1\""); - test_parse_query_to_ast_helper("+abc:toto", "+(abc:\"toto\")"); - test_parse_query_to_ast_helper("(+abc:toto -titi)", "(+(abc:\"toto\") -(\"titi\"))"); - test_parse_query_to_ast_helper("-abc:toto", "-(abc:\"toto\")"); - test_parse_query_to_ast_helper("abc:a b", "(abc:\"a\" \"b\")"); + } + + #[test] + fn test_must_clause() { + test_parse_query_to_ast_helper("(+a +b)", "(+\"a\" +\"b\")"); + } + + #[test] + fn test_parse_test_query_plus_a_b_plus_d() { + test_parse_query_to_ast_helper("+(a b) +d", "(+(*\"a\" *\"b\") +\"d\")"); + } + + #[test] + fn test_parse_test_query_other() { + test_parse_query_to_ast_helper("(+a +b) d", "(*(+\"a\" +\"b\") *\"d\")"); + test_parse_query_to_ast_helper("+abc:toto", "abc:\"toto\""); + test_parse_query_to_ast_helper("(+abc:toto -titi)", "(+abc:\"toto\" -\"titi\")"); + test_parse_query_to_ast_helper("-abc:toto", "(-abc:\"toto\")"); + test_parse_query_to_ast_helper("abc:a b", "(*abc:\"a\" *\"b\")"); test_parse_query_to_ast_helper("abc:\"a b\"", "abc:\"a b\""); test_parse_query_to_ast_helper("foo:[1 TO 5]", "foo:[\"1\" TO \"5\"]"); + } + + #[test] + fn test_parse_query_with_range() { test_parse_query_to_ast_helper("[1 TO 5]", "[\"1\" TO \"5\"]"); test_parse_query_to_ast_helper("foo:{a TO z}", "foo:{\"a\" TO \"z\"}"); test_parse_query_to_ast_helper("foo:[1 TO toto}", "foo:[\"1\" TO \"toto\"}"); diff --git a/query-grammar/src/user_input_ast.rs b/query-grammar/src/user_input_ast.rs index 5ff841869..1fa56c845 100644 --- a/query-grammar/src/user_input_ast.rs +++ b/query-grammar/src/user_input_ast.rs @@ -85,15 +85,14 @@ impl UserInputBound { } pub enum UserInputAST { - Clause(Vec), - Unary(Occur, Box), + Clause(Vec<(Option, UserInputAST)>), Leaf(Box), Boost(Box, f32), } impl UserInputAST { pub fn unary(self, occur: Occur) -> UserInputAST { - UserInputAST::Unary(occur, Box::new(self)) + UserInputAST::Clause(vec![(Some(occur), self)]) } fn compose(occur: Occur, asts: Vec) -> UserInputAST { @@ -104,7 +103,7 @@ impl UserInputAST { } else { UserInputAST::Clause( asts.into_iter() - .map(|ast: UserInputAST| ast.unary(occur)) + .map(|ast: UserInputAST| (Some(occur), ast)) .collect::>(), ) } @@ -135,25 +134,36 @@ impl From for UserInputAST { } } +fn print_occur_ast( + occur_opt: Option, + ast: &UserInputAST, + formatter: &mut fmt::Formatter, +) -> fmt::Result { + if let Some(occur) = occur_opt { + write!(formatter, "{}{:?}", occur, ast)?; + } else { + write!(formatter, "*{:?}", ast)?; + } + Ok(()) +} + impl fmt::Debug for UserInputAST { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { match *self { UserInputAST::Clause(ref subqueries) => { if subqueries.is_empty() { write!(formatter, "")?; } else { write!(formatter, "(")?; - write!(formatter, "{:?}", &subqueries[0])?; + print_occur_ast(subqueries[0].0, &subqueries[0].1, formatter)?; for subquery in &subqueries[1..] { - write!(formatter, " {:?}", subquery)?; + write!(formatter, " ")?; + print_occur_ast(subquery.0, &subquery.1, formatter)?; } write!(formatter, ")")?; } Ok(()) } - UserInputAST::Unary(ref occur, ref subquery) => { - write!(formatter, "{}({:?})", occur, subquery) - } UserInputAST::Leaf(ref subquery) => write!(formatter, "{:?}", subquery), UserInputAST::Boost(ref leaf, boost) => write!(formatter, "({:?})^{}", leaf, boost), } diff --git a/src/query/boolean_query/mod.rs b/src/query/boolean_query/mod.rs index f61072b00..1c6a341ef 100644 --- a/src/query/boolean_query/mod.rs +++ b/src/query/boolean_query/mod.rs @@ -31,24 +31,11 @@ mod tests { // writing the segment let mut index_writer = index.writer_with_num_threads(1, 3_000_000).unwrap(); { - let doc = doc!(text_field => "a b c"); - index_writer.add_document(doc); - } - { - let doc = doc!(text_field => "a c"); - index_writer.add_document(doc); - } - { - let doc = doc!(text_field => "b c"); - index_writer.add_document(doc); - } - { - let doc = doc!(text_field => "a b c d"); - index_writer.add_document(doc); - } - { - let doc = doc!(text_field => "d"); - index_writer.add_document(doc); + index_writer.add_document(doc!(text_field => "a b c")); + index_writer.add_document(doc!(text_field => "a c")); + index_writer.add_document(doc!(text_field => "b c")); + index_writer.add_document(doc!(text_field => "a b c d")); + index_writer.add_document(doc!(text_field => "d")); } assert!(index_writer.commit().is_ok()); } diff --git a/src/query/query_parser/query_parser.rs b/src/query/query_parser/query_parser.rs index 6055cc2ce..ec8bdc99e 100644 --- a/src/query/query_parser/query_parser.rs +++ b/src/query/query_parser/query_parser.rs @@ -55,8 +55,8 @@ pub enum QueryParserError { /// The tokenizer for the given field is unknown /// The two argument strings are the name of the field, the name of the tokenizer #[fail( - display = "The tokenizer '{:?}' for the field '{:?}' is unknown", - _0, _1 + display = "The tokenizer '{:?}' for the field '{:?}' is unknown", + _0, _1 )] UnknownTokenizer(String, String), /// The query contains a range query with a phrase as one of the bounds. @@ -174,6 +174,16 @@ pub struct QueryParser { boost: HashMap, } +fn all_negative(ast: &LogicalAST) -> bool { + match ast { + LogicalAST::Leaf(_) => false, + LogicalAST::Boost(ref child_ast, _) => all_negative(&*child_ast), + LogicalAST::Clause(children) => children + .iter() + .all(|(ref occur, child)| (*occur == Occur::MustNot) || all_negative(child)), + } +} + impl QueryParser { /// Creates a `QueryParser`, given /// * schema - index Schema @@ -253,8 +263,13 @@ impl QueryParser { &self, user_input_ast: UserInputAST, ) -> Result { - let (occur, ast) = self.compute_logical_ast_with_occur(user_input_ast)?; - if occur == Occur::MustNot { + let ast = self.compute_logical_ast_with_occur(user_input_ast)?; + if let LogicalAST::Clause(children) = &ast { + if children.is_empty() { + return Ok(ast); + } + } + if all_negative(&ast) { return Err(QueryParserError::AllButQueryForbidden); } Ok(ast) @@ -410,31 +425,23 @@ impl QueryParser { fn compute_logical_ast_with_occur( &self, user_input_ast: UserInputAST, - ) -> Result<(Occur, LogicalAST), QueryParserError> { + ) -> Result { match user_input_ast { UserInputAST::Clause(sub_queries) => { let default_occur = self.default_occur(); let mut logical_sub_queries: Vec<(Occur, LogicalAST)> = Vec::new(); - for sub_query in sub_queries { - let (occur, sub_ast) = self.compute_logical_ast_with_occur(sub_query)?; - let new_occur = Occur::compose(default_occur, occur); - logical_sub_queries.push((new_occur, sub_ast)); + for (occur_opt, sub_ast) in sub_queries { + let sub_ast = self.compute_logical_ast_with_occur(sub_ast)?; + let occur = occur_opt.unwrap_or(default_occur); + logical_sub_queries.push((occur, sub_ast)); } - Ok((Occur::Should, LogicalAST::Clause(logical_sub_queries))) - } - UserInputAST::Unary(left_occur, subquery) => { - let (right_occur, logical_sub_queries) = - self.compute_logical_ast_with_occur(*subquery)?; - Ok((Occur::compose(left_occur, right_occur), logical_sub_queries)) + Ok(LogicalAST::Clause(logical_sub_queries)) } UserInputAST::Boost(ast, boost) => { - let (occur, ast_without_occur) = self.compute_logical_ast_with_occur(*ast)?; - Ok((occur, ast_without_occur.boost(boost))) - } - UserInputAST::Leaf(leaf) => { - let result_ast = self.compute_logical_ast_from_leaf(*leaf)?; - Ok((Occur::Should, result_ast)) + let ast = self.compute_logical_ast_with_occur(*ast)?; + Ok(ast.boost(boost)) } + UserInputAST::Leaf(leaf) => self.compute_logical_ast_from_leaf(*leaf), } } @@ -782,6 +789,20 @@ mod test { ); } + #[test] + fn test_parse_query_to_ast_ab_c() { + test_parse_query_to_logical_ast_helper( + "(+title:a +title:b) title:c", + "((+Term(field=0,bytes=[97]) +Term(field=0,bytes=[98])) Term(field=0,bytes=[99]))", + false, + ); + test_parse_query_to_logical_ast_helper( + "(+title:a +title:b) title:c", + "(+(+Term(field=0,bytes=[97]) +Term(field=0,bytes=[98])) +Term(field=0,bytes=[99]))", + true, + ); + } + #[test] pub fn test_parse_query_to_ast_single_term() { test_parse_query_to_logical_ast_helper( @@ -801,11 +822,13 @@ mod test { Term(field=1,bytes=[116, 105, 116, 105])))", false, ); - assert_eq!( - parse_query_to_logical_ast("-title:toto", false) - .err() - .unwrap(), - QueryParserError::AllButQueryForbidden + } + + #[test] + fn test_single_negative_term() { + assert_matches!( + parse_query_to_logical_ast("-title:toto", false), + Err(QueryParserError::AllButQueryForbidden) ); } @@ -965,6 +988,18 @@ mod test { assert!(query_parser.parse_query("with_stop_words:the").is_ok()); } + #[test] + pub fn test_parse_query_single_negative_term_through_error() { + assert_matches!( + parse_query_to_logical_ast("-title:toto", true), + Err(QueryParserError::AllButQueryForbidden) + ); + assert_matches!( + parse_query_to_logical_ast("-title:toto", false), + Err(QueryParserError::AllButQueryForbidden) + ); + } + #[test] pub fn test_parse_query_to_ast_conjunction() { test_parse_query_to_logical_ast_helper( @@ -984,12 +1019,6 @@ mod test { Term(field=1,bytes=[116, 105, 116, 105])))", true, ); - assert_eq!( - parse_query_to_logical_ast("-title:toto", true) - .err() - .unwrap(), - QueryParserError::AllButQueryForbidden - ); test_parse_query_to_logical_ast_helper( "title:a b", "(+Term(field=0,bytes=[97]) \ @@ -1013,4 +1042,27 @@ mod test { false ); } + + #[test] + fn test_and_default_regardless_of_default_conjunctive() { + for &default_conjunction in &[false, true] { + test_parse_query_to_logical_ast_helper( + "title:a AND title:b", + "(+Term(field=0,bytes=[97]) +Term(field=0,bytes=[98]))", + default_conjunction + ); + } + } + + #[test] + fn test_or_default_conjunctive() { + for &default_conjunction in &[false, true] { + test_parse_query_to_logical_ast_helper( + "title:a OR title:b", + "(Term(field=0,bytes=[97]) Term(field=0,bytes=[98]))", + default_conjunction + ); + } + } + }