-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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 nGram
and edgeNGram
token filter names (#38911)
#39070
Conversation
In elastic#30209 we deprecated the camel case `nGram` filter name in favour of `ngram` and did the same for `edgeNGram` and `edge_ngram`. Using these names has been deprecated since 6.4 and is issuing deprecation warnings since then. I think we can remove these filters in 8.0. In a backport of this PR I would change what was a dreprecation warning from 6.4. to an error starting with new indices created in 7.0.
Pinging @elastic/es-search |
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.
Thanks @cbuescher , I left one question.
...es/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java
Outdated
Show resolved
Hide resolved
...es/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java
Outdated
Show resolved
Hide resolved
@jimczi thanks, I pushed a commit adressing your comments |
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.
I left minor comments but feel free to merge without another review.
...es/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java
Outdated
Show resolved
Hide resolved
...es/analysis-common/src/main/java/org/elasticsearch/analysis/common/CommonAnalysisPlugin.java
Outdated
Show resolved
Hide resolved
In #30209 we deprecated the camel case `nGram` filter name in favour of `ngram` and did the same for `edgeNGram` and `edge_ngram` and we are removing those names in 8.0. This change disallows using the deprecated names for new indices created in 7.0 by throwing an error if these filters are used. Relates to #38911
In #30209 we deprecated the camel case
nGram
token filter in favour ofngram
anddeprecated
edgeNGram
in favour ofedge_ngram
. Using these names has been deprecatedsince 6.4, issuing deprecation warnings on new indices since then. In addition to remove the filter
in 8.0 completely (see ##38911) we should not allow index creation using these filter names
starting with 8.0. This PR adds throwing an IAE when this filter name is used in indices created from
7.0 on.