-
Notifications
You must be signed in to change notification settings - Fork 451
allow querying non existing fields #5308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
quickwit/quickwit-integration-tests/src/tests/update_tests/doc_mapping_tests.rs
Outdated
Show resolved
Hide resolved
we may make so we accept query on non-existing field only when lenient is true. This is different from what ES does, but there seems to be a consensus among us that what ES does here is not a behaviour we like (being too lenient by default) |
fd239da
to
564e26f
Compare
@@ -45,8 +45,8 @@ pub(crate) struct MatchQueryParams { | |||
// Regardless of this option Quickwit behaves in elasticsearch definition of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you edit the comment?
#[serde(default, rename = "lenient")] | ||
_lenient: bool, | ||
#[serde(default)] | ||
lenient: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as for match query. we need to edit the comment.
@@ -99,6 +105,7 @@ fn convert_user_input_ast_to_query_ast( | |||
user_input_ast: UserInputAst, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add some short documentation for this method?
Description
fix #5302
the spec around update is a bit odd: we can search inside deleted fields (validation is okay with inexistant field, and mapping the query per-split actually find the field for older splits), but we don't return these fields
How was this PR tested?
updated ut, added it