allow querying non existing fields (#5308)

* make integration test idempotent * ignore missing field on lenient query
quickwit-oss · Aug 30, 2024 · d684b92 · d684b92
1 parent 489c9d6
commit d684b92
Show file tree

Hide file tree

Showing 21 changed files with 123 additions and 38 deletions.
diff --git a/quickwit/quickwit-doc-mapper/src/doc_mapper.rs b/quickwit/quickwit-doc-mapper/src/doc_mapper.rs
@@ -361,6 +361,7 @@ mod tests {
             user_text: "json_field.toto.titi:hello".to_string(),
             default_fields: None,
             default_operator: BooleanOperand::And,
+            lenient: false,
         }
         .parse_user_query(&[])
         .unwrap();

diff --git a/quickwit/quickwit-doc-mapper/src/tag_pruning.rs b/quickwit/quickwit-doc-mapper/src/tag_pruning.rs
@@ -398,6 +398,7 @@ mod test {
             user_text: user_query.to_string(),
             default_fields: None,
             default_operator: BooleanOperand::Or,
+            lenient: false,
         }
         .into();
         let parsed_query_ast = query_ast.parse_user_query(&[]).unwrap();

diff --git a/quickwit/quickwit-jaeger/src/lib.rs b/quickwit/quickwit-jaeger/src/lib.rs
@@ -1182,7 +1182,8 @@ mod tests {
                 quickwit_query::query_ast::UserInputQuery {
                     user_text: "query".to_string(),
                     default_fields: None,
-                    default_operator: quickwit_query::BooleanOperand::And
+                    default_operator: quickwit_query::BooleanOperand::And,
+                    lenient: false,
                 }
                 .into()
             );

diff --git a/quickwit/quickwit-metastore/test-data/file-backed-index/v0.9.expected.json b/quickwit/quickwit-metastore/test-data/file-backed-index/v0.9.expected.json
@@ -190,7 +190,7 @@
       "opstamp": 10,
       "delete_query": {
         "index_uid": "my-index:00000000000000000000000001",
-        "query_ast": "{\"type\":\"bool\",\"must\":[{\"type\":\"full_text\",\"field\":\"body\",\"text\":\"Harry\",\"params\":{\"mode\":{\"type\":\"phrase_fallback_to_intersection\"}}},{\"type\":\"full_text\",\"field\":\"body\",\"text\":\"Potter\",\"params\":{\"mode\":{\"type\":\"phrase_fallback_to_intersection\"}}}]}"
+        "query_ast": "{\"type\":\"bool\",\"must\":[{\"type\":\"full_text\",\"field\":\"body\",\"text\":\"Harry\",\"params\":{\"mode\":{\"type\":\"phrase_fallback_to_intersection\"}},\"lenient\":false},{\"type\":\"full_text\",\"field\":\"body\",\"text\":\"Potter\",\"params\":{\"mode\":{\"type\":\"phrase_fallback_to_intersection\"}},\"lenient\":false}]}"
       }
     }
   ]

diff --git a/quickwit/quickwit-metastore/test-data/file-backed-index/v0.9.json b/quickwit/quickwit-metastore/test-data/file-backed-index/v0.9.json
@@ -190,7 +190,7 @@
       "opstamp": 10,
       "delete_query": {
         "index_uid": "my-index:00000000000000000000000001",
-        "query_ast": "{\"type\":\"bool\",\"must\":[{\"type\":\"full_text\",\"field\":\"body\",\"text\":\"Harry\",\"params\":{\"mode\":{\"type\":\"phrase_fallback_to_intersection\"}}},{\"type\":\"full_text\",\"field\":\"body\",\"text\":\"Potter\",\"params\":{\"mode\":{\"type\":\"phrase_fallback_to_intersection\"}}}]}"
+        "query_ast": "{\"type\":\"bool\",\"must\":[{\"type\":\"full_text\",\"field\":\"body\",\"text\":\"Harry\",\"params\":{\"mode\":{\"type\":\"phrase_fallback_to_intersection\"}},\"lenient\":false},{\"type\":\"full_text\",\"field\":\"body\",\"text\":\"Potter\",\"params\":{\"mode\":{\"type\":\"phrase_fallback_to_intersection\"}},\"lenient\":false}]}"
       }
     }
   ]

diff --git a/quickwit/quickwit-query/src/elastic_query_dsl/match_bool_prefix.rs b/quickwit/quickwit-query/src/elastic_query_dsl/match_bool_prefix.rs
@@ -48,6 +48,7 @@ impl ConvertibleToQueryAst for MatchBoolPrefixQuery {
             field: self.field,
             text: self.params.query,
             params: full_text_params,
+            lenient: self.params.lenient,
         }))
     }
 }

diff --git a/quickwit/quickwit-query/src/elastic_query_dsl/match_phrase_query.rs b/quickwit/quickwit-query/src/elastic_query_dsl/match_phrase_query.rs
@@ -59,6 +59,7 @@ impl ConvertibleToQueryAst for MatchPhraseQuery {
             field: self.field,
             text: self.params.query,
             params: full_text_params,
+            lenient: false,
         }))
     }
 }
@@ -159,6 +160,7 @@ mod tests {
             field,
             text,
             params,
+            lenient: _,
         }) = ast
         else {
             panic!()

diff --git a/quickwit/quickwit-query/src/elastic_query_dsl/match_query.rs b/quickwit/quickwit-query/src/elastic_query_dsl/match_query.rs
@@ -42,11 +42,11 @@ pub(crate) struct MatchQueryParams {
     pub(crate) operator: BooleanOperand,
     #[serde(default)]
     pub(crate) zero_terms_query: MatchAllOrNone,
-    // Regardless of this option Quickwit behaves in elasticsearch definition of
-    // lenient. We include this property here just to accept user queries containing
-    // this option.
-    #[serde(default, rename = "lenient")]
-    pub(crate) _lenient: bool,
+    // Quickwit and Elastic have different notions of lenient. For us, it means it's okay to
+    // disregard part of the query where which uses non-existing collumn (which Elastic does by
+    // default). For Elastic, it covers type errors (searching text in an integer field).
+    #[serde(default)]
+    pub(crate) lenient: bool,
 }
 
 impl ConvertibleToQueryAst for MatchQuery {
@@ -60,6 +60,7 @@ impl ConvertibleToQueryAst for MatchQuery {
             field: self.field,
             text: self.params.query,
             params: full_text_params,
+            lenient: self.params.lenient,
         }))
     }
 }
@@ -88,7 +89,7 @@ impl From<String> for MatchQueryParams {
             query,
             zero_terms_query: Default::default(),
             operator: Default::default(),
-            _lenient: false,
+            lenient: false,
         }
     }
 }
@@ -137,14 +138,15 @@ mod tests {
                 query: "hello".to_string(),
                 operator: BooleanOperand::And,
                 zero_terms_query: crate::MatchAllOrNone::MatchAll,
-                _lenient: false,
+                lenient: false,
             },
         };
         let ast = match_query.convert_to_query_ast().unwrap();
         let QueryAst::FullText(FullTextQuery {
             field,
             text,
             params,
+            lenient: _,
         }) = ast
         else {
             panic!()

diff --git a/quickwit/quickwit-query/src/elastic_query_dsl/multi_match.rs b/quickwit/quickwit-query/src/elastic_query_dsl/multi_match.rs
@@ -30,7 +30,7 @@ use crate::elastic_query_dsl::phrase_prefix_query::{
 };
 use crate::elastic_query_dsl::{ConvertibleToQueryAst, ElasticQueryDslInner};
 
-/// Multi match queries are a bit odd. They end up being expanded into another type query of query.
+/// Multi match queries are a bit odd. They end up being expanded into another type of query.
 /// In Quickwit, we operate this expansion in generic way at the time of deserialization.
 #[derive(Deserialize, Debug, Eq, PartialEq, Clone)]
 #[serde(try_from = "MultiMatchQueryForDeserialization")]
@@ -48,11 +48,11 @@ struct MultiMatchQueryForDeserialization {
     #[serde_as(deserialize_as = "OneOrMany<_, PreferMany>")]
     #[serde(default)]
     fields: Vec<String>,
-    // Regardless of this option Quickwit behaves in elasticsearch definition of
-    // lenient. We include this property here just to accept user queries containing
-    // this option.
-    #[serde(default, rename = "lenient")]
-    _lenient: bool,
+    // Quickwit and Elastic have different notions of lenient. For us, it means it's okay to
+    // disregard part of the query where which uses non-existing collumn (which Elastic does by
+    // default). For Elastic, it covers type errors (searching text in an integer field).
+    #[serde(default)]
+    lenient: bool,
 }
 
 fn deserialize_match_query_for_one_field(
@@ -198,7 +198,7 @@ mod tests {
                         query: "quick brown fox".to_string(),
                         operator: crate::BooleanOperand::Or,
                         zero_terms_query: Default::default(),
-                        _lenient: false,
+                        lenient: false,
                     },
                 }
                 .into(),
@@ -208,7 +208,7 @@ mod tests {
                         query: "quick brown fox".to_string(),
                         operator: crate::BooleanOperand::Or,
                         zero_terms_query: Default::default(),
-                        _lenient: false,
+                        lenient: false,
                     },
                 }
                 .into(),
@@ -228,7 +228,7 @@ mod tests {
                         query: "quick brown fox".to_string(),
                         operator: crate::BooleanOperand::Or,
                         zero_terms_query: Default::default(),
-                        _lenient: false,
+                        lenient: false,
                     },
                 }
                 .into(),
@@ -238,7 +238,7 @@ mod tests {
                         query: "quick brown fox".to_string(),
                         operator: crate::BooleanOperand::Or,
                         zero_terms_query: Default::default(),
-                        _lenient: false,
+                        lenient: false,
                     },
                 }
                 .into(),
@@ -258,7 +258,7 @@ mod tests {
                         query: "quick brown fox".to_string(),
                         operator: crate::BooleanOperand::Or,
                         zero_terms_query: Default::default(),
-                        _lenient: false,
+                        lenient: false,
                     },
                 }
                 .into(),
@@ -268,7 +268,7 @@ mod tests {
                         query: "quick brown fox".to_string(),
                         operator: crate::BooleanOperand::Or,
                         zero_terms_query: Default::default(),
-                        _lenient: false,
+                        lenient: false,
                     },
                 }
                 .into(),
@@ -350,7 +350,7 @@ mod tests {
                         query: "quick brown".to_string(),
                         operator: crate::BooleanOperand::Or,
                         zero_terms_query: Default::default(),
-                        _lenient: false,
+                        lenient: false,
                     },
                 }
                 .into(),
@@ -360,7 +360,7 @@ mod tests {
                         query: "quick brown".to_string(),
                         operator: crate::BooleanOperand::Or,
                         zero_terms_query: Default::default(),
-                        _lenient: false,
+                        lenient: false,
                     },
                 }
                 .into(),

diff --git a/quickwit/quickwit-query/src/elastic_query_dsl/query_string_query.rs b/quickwit/quickwit-query/src/elastic_query_dsl/query_string_query.rs
@@ -43,8 +43,8 @@ pub(crate) struct QueryStringQuery {
     // Regardless of this option Quickwit behaves in elasticsearch definition of
     // lenient. We include this property here just to accept user queries containing
     // this option.
-    #[serde(default, rename = "lenient")]
-    _lenient: bool,
+    #[serde(default)]
+    lenient: bool,
 }
 
 impl ConvertibleToQueryAst for QueryStringQuery {
@@ -60,6 +60,7 @@ impl ConvertibleToQueryAst for QueryStringQuery {
             user_text: self.query,
             default_fields,
             default_operator: self.default_operator,
+            lenient: self.lenient,
         };
         Ok(user_text_query.into())
     }
@@ -79,7 +80,7 @@ mod tests {
             default_operator: crate::BooleanOperand::Or,
             default_field: None,
             boost: None,
-            _lenient: false,
+            lenient: false,
         };
         let QueryAst::UserInput(user_input_query) =
             query_string_query.convert_to_query_ast().unwrap()
@@ -101,7 +102,7 @@ mod tests {
             default_operator: crate::BooleanOperand::Or,
             default_field: Some("hello".to_string()),
             boost: None,
-            _lenient: false,
+            lenient: false,
         };
         let QueryAst::UserInput(user_input_query) =
             query_string_query.convert_to_query_ast().unwrap()
@@ -123,7 +124,7 @@ mod tests {
             default_operator: crate::BooleanOperand::Or,
             default_field: Some("hello".to_string()),
             boost: None,
-            _lenient: false,
+            lenient: false,
         };
         let err_msg = query_string_query
             .convert_to_query_ast()
@@ -140,7 +141,7 @@ mod tests {
             default_field: None,
             default_operator: crate::BooleanOperand::And,
             boost: None,
-            _lenient: false,
+            lenient: false,
         };
         let QueryAst::UserInput(user_input_query) =
             query_string_query.convert_to_query_ast().unwrap()
@@ -158,7 +159,7 @@ mod tests {
             default_field: None,
             default_operator: crate::BooleanOperand::Or,
             boost: None,
-            _lenient: false,
+            lenient: false,
         };
         let QueryAst::UserInput(user_input_query) =
             query_string_query.convert_to_query_ast().unwrap()
@@ -177,7 +178,7 @@ mod tests {
             default_field: None,
             default_operator: crate::BooleanOperand::Or,
             boost: None,
-            _lenient: false,
+            lenient: false,
         };
         let QueryAst::UserInput(user_input_query) =
             query_string_query.convert_to_query_ast().unwrap()
@@ -200,7 +201,8 @@ mod tests {
         assert!(matches!(query_ast, QueryAst::UserInput(UserInputQuery {
             user_text,
             default_fields,
-            default_operator
+            default_operator,
+            lenient: _,
         }) if user_text == "hello world"
             && default_operator == BooleanOperand::Or
             && default_fields == Some(vec!["text".to_string()])));

diff --git a/quickwit/quickwit-query/src/query_ast/full_text_query.rs b/quickwit/quickwit-query/src/query_ast/full_text_query.rs
@@ -227,6 +227,7 @@ pub struct FullTextQuery {
     pub field: String,
     pub text: String,
     pub params: FullTextParams,
+    pub lenient: bool,
 }
 
 impl From<FullTextQuery> for QueryAst {
@@ -249,6 +250,7 @@ impl BuildTantivyAst for FullTextQuery {
             &self.params,
             schema,
             tokenizer_manager,
+            self.lenient,
         )
     }
 }
@@ -323,6 +325,7 @@ mod tests {
                 mode: BooleanOperand::And.into(),
                 zero_terms_query: crate::MatchAllOrNone::MatchAll,
             },
+            lenient: false,
         };
         let mut schema_builder = Schema::builder();
         schema_builder.add_text_field("body", TEXT);
@@ -348,6 +351,7 @@ mod tests {
                 mode: FullTextMode::Phrase { slop: 1 },
                 zero_terms_query: crate::MatchAllOrNone::MatchAll,
             },
+            lenient: false,
         };
         let mut schema_builder = Schema::builder();
         schema_builder.add_text_field("body", TEXT);
@@ -378,6 +382,7 @@ mod tests {
                 mode: FullTextMode::Phrase { slop: 1 },
                 zero_terms_query: crate::MatchAllOrNone::MatchAll,
             },
+            lenient: false,
         };
         let mut schema_builder = Schema::builder();
         schema_builder.add_text_field("body", TEXT);
@@ -407,6 +412,7 @@ mod tests {
                 mode: BooleanOperand::And.into(),
                 zero_terms_query: crate::MatchAllOrNone::MatchAll,
             },
+            lenient: false,
         };
         let mut schema_builder = Schema::builder();
         schema_builder.add_text_field("body", TEXT);

diff --git a/quickwit/quickwit-query/src/query_ast/mod.rs b/quickwit/quickwit-query/src/query_ast/mod.rs
@@ -316,6 +316,7 @@ pub fn query_ast_from_user_text(user_text: &str, default_fields: Option<Vec<Stri
         user_text: user_text.to_string(),
         default_fields,
         default_operator: BooleanOperand::And,
+        lenient: false,
     }
     .into()
 }
@@ -334,6 +335,7 @@ mod tests {
             user_text: "*".to_string(),
             default_fields: Default::default(),
             default_operator: Default::default(),
+            lenient: false,
         }
         .into();
         let schema = tantivy::schema::Schema::builder().build();
@@ -357,6 +359,7 @@ mod tests {
             user_text: "*".to_string(),
             default_fields: Default::default(),
             default_operator: Default::default(),
+            lenient: false,
         }
         .into();
         let query_ast_with_parsed_user_query: QueryAst = query_ast.parse_user_query(&[]).unwrap();
@@ -378,6 +381,7 @@ mod tests {
             user_text: "*".to_string(),
             default_fields: Default::default(),
             default_operator: Default::default(),
+            lenient: false,
         }
         .into();
         let bool_query_ast: QueryAst = BoolQuery {
@@ -412,6 +416,7 @@ mod tests {
             user_text: "field:hello field:toto".to_string(),
             default_fields: None,
             default_operator: crate::BooleanOperand::And,
+            lenient: false,
         }
         .parse_user_query(&[])
         .unwrap();
@@ -427,6 +432,7 @@ mod tests {
             user_text: "field:hello field:toto".to_string(),
             default_fields: None,
             default_operator: crate::BooleanOperand::Or,
+            lenient: false,
         }
         .parse_user_query(&[])
         .unwrap();