From c84146e693cb6978179a4c804cc944a3e5512274 Mon Sep 17 00:00:00 2001 From: MagellaX Date: Mon, 23 Jun 2025 01:50:50 +0530 Subject: [PATCH 1/4] feat(query): make empty-query behavior configurable in QueryParser (#386) --- src/query/query_parser/query_parser.rs | 39 +++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/src/query/query_parser/query_parser.rs b/src/query/query_parser/query_parser.rs index 3526209c3e..9d39b69396 100644 --- a/src/query/query_parser/query_parser.rs +++ b/src/query/query_parser/query_parser.rs @@ -206,6 +206,9 @@ pub struct QueryParser { tokenizer_manager: TokenizerManager, boost: FxHashMap, fuzzy: FxHashMap, + /// If true, an empty query (e.g. "" or only whitespace) will match all + /// documents instead of matching none. + empty_query_match_all: bool, } #[derive(Clone)] @@ -260,6 +263,7 @@ impl QueryParser { conjunction_by_default: false, boost: Default::default(), fuzzy: Default::default(), + empty_query_match_all: false, } } @@ -320,13 +324,26 @@ impl QueryParser { ); } + /// Configure the behaviour of an empty query (e.g. an empty string or only whitespace). + /// + /// If `should_match_all` is `true`, an empty query will match all documents in the index. + /// Otherwise (the default), an empty query matches no documents. + pub fn set_empty_query_match_all(&mut self, should_match_all: bool) { + self.empty_query_match_all = should_match_all; + } + + /// Returns the current behaviour for empty queries. + pub fn get_empty_query_match_all(&self) -> bool { + self.empty_query_match_all + } + /// Parse a query /// /// Note that `parse_query` returns an error if the input /// is not a valid query. pub fn parse_query(&self, query: &str) -> Result, QueryParserError> { let logical_ast = self.parse_query_to_logical_ast(query)?; - Ok(convert_to_query(&self.fuzzy, logical_ast)) + Ok(self.logical_ast_to_query(logical_ast)) } /// Parse a query leniently @@ -339,7 +356,7 @@ impl QueryParser { /// In case it encountered such issues, they are reported as a Vec of errors. pub fn parse_query_lenient(&self, query: &str) -> (Box, Vec) { let (logical_ast, errors) = self.parse_query_to_logical_ast_lenient(query); - (convert_to_query(&self.fuzzy, logical_ast), errors) + (self.logical_ast_to_query(logical_ast), errors) } /// Build a query from an already parsed user input AST @@ -355,7 +372,7 @@ impl QueryParser { if !err.is_empty() { return Err(err.swap_remove(0)); } - Ok(convert_to_query(&self.fuzzy, logical_ast)) + Ok(self.logical_ast_to_query(logical_ast)) } /// Build leniently a query from an already parsed user input AST. @@ -366,7 +383,7 @@ impl QueryParser { user_input_ast: UserInputAst, ) -> (Box, Vec) { let (logical_ast, errors) = self.compute_logical_ast_lenient(user_input_ast); - (convert_to_query(&self.fuzzy, logical_ast), errors) + (self.logical_ast_to_query(logical_ast), errors) } /// Parse the user query into an AST. @@ -856,6 +873,20 @@ impl QueryParser { ), } } + + /// Convert a logical AST into a runnable `Query`, taking into account the parser configuration. + fn logical_ast_to_query(&self, logical_ast: LogicalAst) -> Box { + match trim_ast(logical_ast) { + Some(trimmed_ast) => convert_to_query(&self.fuzzy, trimmed_ast), + None => { + if self.empty_query_match_all { + Box::new(AllQuery) + } else { + Box::new(EmptyQuery) + } + } + } + } } fn convert_literal_to_query( From bf560ac927a3d291a40874ad67152f42032c9716 Mon Sep 17 00:00:00 2001 From: MagellaX Date: Tue, 12 Aug 2025 01:11:12 +0530 Subject: [PATCH 2/4] feat(query): decide empty-query at user input AST level; add set/get_empty_query_match_all; keep default EmptyQuery; handle lenient parse errors (#386) --- src/query/query_parser/query_parser.rs | 55 +++++++++++++++++++++----- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/src/query/query_parser/query_parser.rs b/src/query/query_parser/query_parser.rs index 9d39b69396..59744e09bb 100644 --- a/src/query/query_parser/query_parser.rs +++ b/src/query/query_parser/query_parser.rs @@ -337,12 +337,35 @@ impl QueryParser { self.empty_query_match_all } + #[inline] + fn empty_query_result(&self) -> Box { + if self.empty_query_match_all { + Box::new(AllQuery) + } else { + Box::new(EmptyQuery) + } + } + + #[inline] + fn is_user_input_ast_empty(user_input_ast: &UserInputAst) -> bool { + matches!(user_input_ast, UserInputAst::Clause(clauses) if clauses.is_empty()) + } + /// Parse a query /// /// Note that `parse_query` returns an error if the input /// is not a valid query. pub fn parse_query(&self, query: &str) -> Result, QueryParserError> { - let logical_ast = self.parse_query_to_logical_ast(query)?; + // Parse to user input AST first so we can decide on empty-query behaviour early + let user_input_ast = query_grammar::parse_query(query) + .map_err(|_| QueryParserError::SyntaxError(query.to_string()))?; + if Self::is_user_input_ast_empty(&user_input_ast) { + return Ok(self.empty_query_result()); + } + let (logical_ast, mut err) = self.compute_logical_ast_lenient(user_input_ast); + if !err.is_empty() { + return Err(err.swap_remove(0)); + } Ok(self.logical_ast_to_query(logical_ast)) } @@ -355,7 +378,21 @@ impl QueryParser { /// /// In case it encountered such issues, they are reported as a Vec of errors. pub fn parse_query_lenient(&self, query: &str) -> (Box, Vec) { - let (logical_ast, errors) = self.parse_query_to_logical_ast_lenient(query); + let (user_input_ast, grammar_errors) = query_grammar::parse_query_lenient(query); + let mut errors: Vec<_> = grammar_errors + .into_iter() + .map(|error| { + QueryParserError::SyntaxError(format!( + "{} at position {}", + error.message, error.pos + )) + }) + .collect(); + if Self::is_user_input_ast_empty(&user_input_ast) { + return (self.empty_query_result(), errors); + } + let (logical_ast, mut ast_errors) = self.compute_logical_ast_lenient(user_input_ast); + errors.append(&mut ast_errors); (self.logical_ast_to_query(logical_ast), errors) } @@ -368,6 +405,9 @@ impl QueryParser { &self, user_input_ast: UserInputAst, ) -> Result, QueryParserError> { + if Self::is_user_input_ast_empty(&user_input_ast) { + return Ok(self.empty_query_result()); + } let (logical_ast, mut err) = self.compute_logical_ast_lenient(user_input_ast); if !err.is_empty() { return Err(err.swap_remove(0)); @@ -382,6 +422,9 @@ impl QueryParser { &self, user_input_ast: UserInputAst, ) -> (Box, Vec) { + if Self::is_user_input_ast_empty(&user_input_ast) { + return (self.empty_query_result(), Vec::new()); + } let (logical_ast, errors) = self.compute_logical_ast_lenient(user_input_ast); (self.logical_ast_to_query(logical_ast), errors) } @@ -878,13 +921,7 @@ impl QueryParser { fn logical_ast_to_query(&self, logical_ast: LogicalAst) -> Box { match trim_ast(logical_ast) { Some(trimmed_ast) => convert_to_query(&self.fuzzy, trimmed_ast), - None => { - if self.empty_query_match_all { - Box::new(AllQuery) - } else { - Box::new(EmptyQuery) - } - } + None => Box::new(EmptyQuery), } } } From 2a5272f6d6b4a5b9879a0ee09f0fc9ea0c62d46d Mon Sep 17 00:00:00 2001 From: Paul Masurel Date: Wed, 3 Sep 2025 10:13:17 +0200 Subject: [PATCH 3/4] Apply suggestions from code review --- src/query/query_parser/query_parser.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/query/query_parser/query_parser.rs b/src/query/query_parser/query_parser.rs index 59744e09bb..2d92cb58a7 100644 --- a/src/query/query_parser/query_parser.rs +++ b/src/query/query_parser/query_parser.rs @@ -337,7 +337,6 @@ impl QueryParser { self.empty_query_match_all } - #[inline] fn empty_query_result(&self) -> Box { if self.empty_query_match_all { Box::new(AllQuery) @@ -346,7 +345,6 @@ impl QueryParser { } } - #[inline] fn is_user_input_ast_empty(user_input_ast: &UserInputAst) -> bool { matches!(user_input_ast, UserInputAst::Clause(clauses) if clauses.is_empty()) } From c81bc873bb7cddbb0aff23234674cf399413f509 Mon Sep 17 00:00:00 2001 From: MagellaX Date: Wed, 3 Sep 2025 14:29:17 +0530 Subject: [PATCH 4/4] test(query_parser): add unit tests for empty-query match-all behavior --- src/query/query_parser/query_parser.rs | 56 ++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/query/query_parser/query_parser.rs b/src/query/query_parser/query_parser.rs index 2d92cb58a7..5be75570bb 100644 --- a/src/query/query_parser/query_parser.rs +++ b/src/query/query_parser/query_parser.rs @@ -1494,6 +1494,62 @@ mod test { assert!(matches!(base64_err, QueryParserError::ExpectedBase64(_))); } + #[test] + fn test_empty_query_match_all_when_enabled_parse_strict() { + let mut query_parser = make_query_parser(); + // enable the new behavior: empty input matches all documents + query_parser.set_empty_query_match_all(true); + + let query = query_parser.parse_query("").unwrap(); + assert_eq!(format!("{query:?}"), "AllQuery"); + + let query_ws = query_parser.parse_query(" ").unwrap(); + assert_eq!(format!("{query_ws:?}"), "AllQuery"); + } + + #[test] + fn test_empty_query_match_all_when_enabled_parse_lenient() { + let mut query_parser = make_query_parser(); + query_parser.set_empty_query_match_all(true); + + let (query, errs) = query_parser.parse_query_lenient(""); + assert!(errs.is_empty()); + assert_eq!(format!("{query:?}"), "AllQuery"); + } + + #[test] + fn test_empty_query_match_all_when_enabled_build_from_ast() { + use query_grammar::UserInputAst; + + let mut query_parser = make_query_parser(); + query_parser.set_empty_query_match_all(true); + + let ast = UserInputAst::Clause(Vec::new()); + let query = query_parser.build_query_from_user_input_ast(ast).unwrap(); + assert_eq!(format!("{query:?}"), "AllQuery"); + } + + #[test] + fn test_empty_query_match_all_when_enabled_build_from_ast_lenient() { + use query_grammar::UserInputAst; + + let mut query_parser = make_query_parser(); + query_parser.set_empty_query_match_all(true); + + let ast = UserInputAst::Clause(Vec::new()); + let (query, errs) = query_parser.build_query_from_user_input_ast_lenient(ast); + assert!(errs.is_empty()); + assert_eq!(format!("{query:?}"), "AllQuery"); + } + + #[test] + fn test_get_set_empty_query_match_all() { + let mut query_parser = make_query_parser(); + assert!(!query_parser.get_empty_query_match_all()); + query_parser.set_empty_query_match_all(true); + assert!(query_parser.get_empty_query_match_all()); + } + #[test] fn test_parse_query_to_ast_ab_c() { test_parse_query_to_logical_ast_helper(