-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Remove lowercase_expanded_terms
and locale
from query-parser options.
#20208
Remove lowercase_expanded_terms
and locale
from query-parser options.
#20208
Conversation
@@ -82,7 +80,8 @@ | |||
private static final ParseField ALLOW_LEADING_WILDCARD_FIELD = new ParseField("allow_leading_wildcard"); | |||
private static final ParseField AUTO_GENERATE_PHRASE_QUERIES_FIELD = new ParseField("auto_generate_phrase_queries"); | |||
private static final ParseField MAX_DETERMINED_STATES_FIELD = new ParseField("max_determined_states"); | |||
private static final ParseField LOWERCASE_EXPANDED_TERMS_FIELD = new ParseField("lowercase_expanded_terms"); | |||
private static final ParseField LOWERCASE_EXPANDED_TERMS_FIELD = new ParseField("lowercase_expanded_terms") | |||
.withAllDeprecated("Decision is now made by the analyzer"); |
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.
Is it really a deprecation ? Shouldn't we try to detect if the parameter is present and throw a nice exception if it happens ?
Left minor comments about deprecation vs exception, LGTM otherwise |
@jimferenczi Thanks for having a look! The reasoning for using deprecation rather than throwing errors is to not force users to fix their queries at the same time as they upgrade the cluster, which is why I made clear in the upgrade notes that these parameters would be ignored. Since we should now do the right thing in all cases, I don't think this would cause bad surprises? |
Fine with me, thanks for the explanation. |
174358e
to
fff283d
Compare
I revived this pull request now that Lucene 6.3 has the bugfix for normalization and DelegatingAnalyzer (https://issues.apache.org/jira/browse/LUCENE-7429). @jimczi Would you mind having another look? |
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
fff283d
to
9044e9b
Compare
…ons. Lucene 6.2 introduces the new `Analyzer.normalize` API, which allows to apply only character-level normalization such as lowercasing or accent folding, which is exactly what is needed to process queries that operate on partial terms such as `prefix`, `wildcard` or `fuzzy` queries. As a consequence, the `lowercase_expanded_terms` option is not necessary anymore. Furthermore, the `locale` option was only needed in order to know how to perform the lowercasing, so this one can be removed as well. Closes elastic#9978
9044e9b
to
b53b848
Compare
@elasticmachine retest this please |
…ons. (#20208) Lucene 6.2 introduces the new `Analyzer.normalize` API, which allows to apply only character-level normalization such as lowercasing or accent folding, which is exactly what is needed to process queries that operate on partial terms such as `prefix`, `wildcard` or `fuzzy` queries. As a consequence, the `lowercase_expanded_terms` option is not necessary anymore. Furthermore, the `locale` option was only needed in order to know how to perform the lowercasing, so this one can be removed as well. Closes #9978
Add `quote_field_suffix` to simple_query_string query Deprecate `lowercase_expanded_terms` and `locale`: elastic/elasticsearch#20208 Add "all field" execution mode to query_string and simple_query_string query: elastic/elasticsearch#20925 Fixes #2792
Add `quote_field_suffix` to simple_query_string query Deprecate `lowercase_expanded_terms` and `locale`: elastic/elasticsearch#20208 Add "all field" execution mode to query_string and simple_query_string query: elastic/elasticsearch#20925 Fixes #2792
* Add missing fields on simple query string query Add `quote_field_suffix` to simple_query_string query Deprecate `lowercase_expanded_terms` and `locale`: elastic/elasticsearch#20208 Add "all field" execution mode to query_string and simple_query_string query: elastic/elasticsearch#20925 Fixes #2792 * Convert to property expressions
* Add missing fields on simple query string query Add `quote_field_suffix` to simple_query_string query Deprecate `lowercase_expanded_terms` and `locale`: elastic/elasticsearch#20208 Add "all field" execution mode to query_string and simple_query_string query: elastic/elasticsearch#20925 Fixes #2792 * Convert to property expressions (cherry picked from commit bef4b9d)
* Add missing fields on simple query string query Add `quote_field_suffix` to simple_query_string query Deprecate `lowercase_expanded_terms` and `locale`: elastic/elasticsearch#20208 Add "all field" execution mode to query_string and simple_query_string query: elastic/elasticsearch#20925 Fixes elastic#2792 * Convert to property expressions (cherry picked from commit bef4b9d)
Lucene 6.2 introduces the new
Analyzer.normalize
API, which allows to applyonly character-level normalization such as lowercasing or accent folding, which
is exactly what is needed to process queries that operate on partial terms such
as
prefix
,wildcard
orfuzzy
queries. As a consequence, thelowercase_expanded_terms
option is not necessary anymore. Furthermore, thelocale
option was only needed in order to know how to perform the lowercasing,so this one can be removed as well.
Closes #9978