-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Fix query_string_query to transform "foo:*" in an exists query on the field name #23433
Conversation
… field name Currently "foo:*" is parsed as prefix query on the field `foo` unless the field is defined in `default_field` or `fields`. This commit fixes this behavior, "foo:*" is now rewritten to an exists query on the field name. This change also removes the assumption that "_all:*" should return all docs. relates #23356
I like the idea! Does it also work if |
I pushed 86d0697 to handle this case |
(FieldNamesFieldMapper.FieldNamesFieldType) context.getMapperService().fullName(FieldNamesFieldMapper.NAME); | ||
if (fieldNamesFieldType.isEnabled() == false) { | ||
// The field_names_field is disabled so we switch to a wildcard query that matches all terms | ||
return new WildcardQuery(new Term(queryText, "*")); |
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.
that will only work on text/keyword fields? I think we need to perform this check up-front in the MappedFieldType?
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.
True but that's also the current behavior. foo:*
does not work if foo
is not a text/keyword field. I don't get what you mean by check up-front ? Should we add another function in the MappedFieldType that returns match all query for the field ? Numbers would return open ranges query, text/keyword wildcard query and we would use this function only if the _field_names
field is disabled ?
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.
Yeah sorry I thought we already had such a method, hence my comment! I think things are good this way, wildcard queries only make sense on text/keyword anyway. Thanks for the explanation!
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.
LGTM
Thanks for reviewing @jpountz |
… field name (#23433) Currently "foo:*" is parsed as prefix query on the field `foo` unless the field is defined in `default_field` or `fields`. This commit fixes this behavior, "foo:*" is now rewritten to an exists query on the field name. This change also removes the assumption that "_all:*" should return all docs. relates #23356
Currently "foo:*" is parsed as prefix query on the field
foo
unless the field is defined indefault_field
orfields
.This commit fixes this behavior, "foo:*" is now rewritten to an exists query on the field name.
This change also removes the assumption that "_all:*" should return all docs.
relates #23356