Fix case sensitivity rules for wildcard queries on text fields #71751
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Wildcard queries on text fields should not apply the fields analyzer to the search query. However, we accidentally enabled this in elastic#53127 by moving the query normalization to the StringFieldType super type. This change fixes this by separating the notion of normalization and case insensitivity (as implemented in the `case_insensitive` flag). This is done because we still need to maintain normalization of the query sting when the wildcard query method on the field type is requested from the `query_string` query parser. Wildcard queries on keyword fields should also continue to apply the fields normalizer, regardless of whether the `case_insensitive` is set, because normalization could involve something else than lowercasing (e.g. substituting umlauts like in the GermanNormalizationFilter). Closes elastic#71403
cbuescher
added
>bug
:Search/Search
Collaborator
|
Pinging @elastic/es-search (Team:Search) |
markharwood
reviewed
Apr 15, 2021
Contributor
markharwood
left a comment
markharwood
left a comment
There was a problem hiding this comment.
Left a couple of comments and it also looks like WildcardFieldMapper is missing a normalizedWildcardQuery override but otherwise LGTM
| assertHitCount(searchResponse, 0L); | ||
|
|
||
| wildCardQuery = wildcardQuery("field1", "bb*"); | ||
| searchResponse = client().prepareSearch().setQuery(wildCardQuery).get(); |
Contributor
There was a problem hiding this comment.
Maybe add a test where the search string is mixed case but set wildCardQuery.caseInsensitive(true)
| boolean caseInsensitive, | ||
| SearchExecutionContext context | ||
| ) { | ||
| return super.wildcardQuery(value, method, caseInsensitive, true, context); |
Contributor
There was a problem hiding this comment.
Maybe calling normalizedWildcardQuery here would make the intent more obvious plus help any tracing back of where we make use of normalized wildcard queries
Member
Author
There was a problem hiding this comment.
Good idea, but I left the caseInsensitive parameter out of the normalizedWildcardQuery signature on purpose, so I need to call the protected method that takes both arguments here. My thinking was that we use normalizedWildcardQuery only from QueryStringQueryParser where we don't have the caseInsensitive option. Maybe you see a different solution?
Contributor
There was a problem hiding this comment.
Ah ok. That makes sense.
cbuescher
pushed a commit
to cbuescher/elasticsearch
that referenced
this pull request
Apr 26, 2021
…ic#71751) Wildcard queries on text fields should not apply the fields analyzer to the search query. However, we accidentally enabled this in elastic#53127 by moving the query normalization to the StringFieldType super type. This change fixes this by separating the notion of normalization and case insensitivity (as implemented in the `case_insensitive` flag). This is done because we still need to maintain normalization of the query sting when the wildcard query method on the field type is requested from the `query_string` query parser. Wildcard queries on keyword fields should also continue to apply the fields normalizer, regardless of whether the `case_insensitive` is set, because normalization could involve something else than lowercasing (e.g. substituting umlauts like in the GermanNormalizationFilter). Closes elastic#71403
cbuescher
pushed a commit
to cbuescher/elasticsearch
that referenced
this pull request
Apr 26, 2021
…ic#71751) Wildcard queries on text fields should not apply the fields analyzer to the search query. However, we accidentally enabled this in elastic#53127 by moving the query normalization to the StringFieldType super type. This change fixes this by separating the notion of normalization and case insensitivity (as implemented in the `case_insensitive` flag). This is done because we still need to maintain normalization of the query sting when the wildcard query method on the field type is requested from the `query_string` query parser. Wildcard queries on keyword fields should also continue to apply the fields normalizer, regardless of whether the `case_insensitive` is set, because normalization could involve something else than lowercasing (e.g. substituting umlauts like in the GermanNormalizationFilter). Closes elastic#71403
cbuescher
pushed a commit
that referenced
this pull request
Apr 26, 2021
… (#72214) Wildcard queries on text fields should not apply the fields analyzer to the search query. However, we accidentally enabled this in #53127 by moving the query normalization to the StringFieldType super type. This change fixes this by separating the notion of normalization and case insensitivity (as implemented in the `case_insensitive` flag). This is done because we still need to maintain normalization of the query sting when the wildcard query method on the field type is requested from the `query_string` query parser. Wildcard queries on keyword fields should also continue to apply the fields normalizer, regardless of whether the `case_insensitive` is set, because normalization could involve something else than lowercasing (e.g. substituting umlauts like in the GermanNormalizationFilter). Closes #71403
cbuescher
pushed a commit
that referenced
this pull request
Apr 26, 2021
… (#72216) Wildcard queries on text fields should not apply the fields analyzer to the search query. However, we accidentally enabled this in #53127 by moving the query normalization to the StringFieldType super type. This change fixes this by separating the notion of normalization and case insensitivity (as implemented in the `case_insensitive` flag). This is done because we still need to maintain normalization of the query sting when the wildcard query method on the field type is requested from the `query_string` query parser. Wildcard queries on keyword fields should also continue to apply the fields normalizer, regardless of whether the `case_insensitive` is set, because normalization could involve something else than lowercasing (e.g. substituting umlauts like in the GermanNormalizationFilter). Closes #71403
cbuescher
pushed a commit
to cbuescher/elasticsearch
that referenced
this pull request
Apr 26, 2021
…ic#71751) (elastic#72214) Wildcard queries on text fields should not apply the fields analyzer to the search query. However, we accidentally enabled this in elastic#53127 by moving the query normalization to the StringFieldType super type. This change fixes this by separating the notion of normalization and case insensitivity (as implemented in the `case_insensitive` flag). This is done because we still need to maintain normalization of the query sting when the wildcard query method on the field type is requested from the `query_string` query parser. Wildcard queries on keyword fields should also continue to apply the fields normalizer, regardless of whether the `case_insensitive` is set, because normalization could involve something else than lowercasing (e.g. substituting umlauts like in the GermanNormalizationFilter). Closes elastic#71403
cbuescher
pushed a commit
that referenced
this pull request
Apr 26, 2021
… (#72224) Wildcard queries on text fields should not apply the fields analyzer to the search query. However, we accidentally enabled this in #53127 by moving the query normalization to the StringFieldType super type. This change fixes this by separating the notion of normalization and case insensitivity (as implemented in the `case_insensitive` flag). This is done because we still need to maintain normalization of the query sting when the wildcard query method on the field type is requested from the `query_string` query parser. Wildcard queries on keyword fields should also continue to apply the fields normalizer, regardless of whether the `case_insensitive` is set, because normalization could involve something else than lowercasing (e.g. substituting umlauts like in the GermanNormalizationFilter). Closes #71403
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Wildcard queries on text fields should not apply the fields analyzer to the
search query. However, we accidentally enabled this in #53127 by moving the
query normalization to the StringFieldType super type. This change fixes this by
separating the notion of normalization and case insensitivity (as implemented in
the
case_insensitiveflag). This is done because we still need to maintainnormalization of the query sting when the wildcard query method on the field type is
requested from the
query_stringquery parser. Wildcard queries on keywordfields should also continue to apply the fields normalizer, regardless of
whether the
case_insensitiveis set, because normalization could involvesomething else than lowercasing (e.g. substituting umlauts like in the
GermanNormalizationFilter).
Closes #71403