Skip to content
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

Deprecate dutch_kp and lovins stemmer as they are removed in Lucene 10 #113143

Merged
merged 13 commits into from
Sep 30, 2024

Conversation

benwtrent
Copy link
Member

Lucene 10 has upgraded its Snowball stemming support, as part of those upgrades, two no longer supported stemmers were removed, KpStemmer and LovinsStemmer. These are dutch_kp and lovins, respectively.

We will deprecate in 8.16 and will remove support for these in a future version.

Copy link
Contributor

Documentation preview:

@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Sep 18, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@elasticsearchmachine
Copy link
Collaborator

Hi @benwtrent, I've created a changelog YAML for you. Note that since this PR is labelled >deprecation, you need to update the changelog YAML to fill out the extended information sections.

title: Deprecate dutch_kp and lovins stemmer as they are removed in Lucene 10
area: Analysis
details: kp, dutch_kp, dutchKp and lovins stemmers are deprecated and will be removed.
impact: These stemmers will be removed and will no longer supported.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "no longer be supported"

Copy link
Contributor

@john-wagster john-wagster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benwtrent benwtrent added the auto-backport-and-merge Automatically create backport pull requests and merge when ready label Sep 24, 2024
@john-wagster
Copy link
Contributor

@elasticmachine update branch

@john-wagster
Copy link
Contributor

@elasticsearchmachine test this please

@benwtrent benwtrent added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Sep 30, 2024
@elasticsearchmachine elasticsearchmachine merged commit 5c840f7 into elastic:main Sep 30, 2024
15 of 16 checks passed
@benwtrent benwtrent deleted the deprecate_kp_lovin branch September 30, 2024 18:04
benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Sep 30, 2024
elastic#113143)

Lucene 10 has upgraded its Snowball stemming support, as part of those
upgrades, two no longer supported stemmers were removed, `KpStemmer` and
`LovinsStemmer`. These are `dutch_kp` and `lovins`, respectively.

We will deprecate in 8.16 and will remove support for these in a future
version.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

@javanna
Copy link
Member

javanna commented Sep 30, 2024

heya the new tests kind of forced me make these stemmers no-op otherwise we'd throw UOE. I made that quick change in 79d3d6a . I also moved the deprecation warnings to be closer with the creation of the no-op token filter. Not sure whether we want to add tests for this etc.

@benwtrent
Copy link
Member Author

@javanna I think that is ok. This PR was focused on getting deprecation warning set up for 8x where behavior is the same just deprecated. I realize this code will have to change for Lucene 10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport-and-merge Automatically create backport pull requests and merge when ready auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >deprecation :Search Relevance/Analysis How text is split into tokens Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants