-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Add max_token_length setting to the CharGroupTokenizer #56860
Add max_token_length setting to the CharGroupTokenizer #56860
Conversation
Pinging @elastic/es-search (:Search/Analysis) |
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.
Hi @ADBalici, thanks for opening this PR, looks great. I left one super small comment but I'd also merge this if you don't get to changing it. Let me know, I will need to run out CI tests on this before merging in any case.
...alysis-common/src/main/java/org/elasticsearch/analysis/common/CharGroupTokenizerFactory.java
Outdated
Show resolved
Hide resolved
@elasticmachine test this please |
...s-common/src/test/java/org/elasticsearch/analysis/common/CharGroupTokenizerFactoryTests.java
Outdated
Show resolved
Hide resolved
Thanks for reviewing this @cbuescher |
@elasticmachine test this please |
@ADBalici I took the liberty to change another checkstyle issue that appeared when running the test, hope you don't mind |
@elasticmachine update branch |
@elasticmachine test this please |
Adds `max_token_length` option to the CharGroupTokenizer. Updates documentation as well to reflect the changes. Closes #56676
@ADBalici test are all good now, I merged to master and the upcoming 7.9 branch. Thanks for your contribution! |
Relates: elastic/elasticsearch#56860 Co-authored-by: Russ Cam <russ.cam@elastic.co>
Relates: elastic/elasticsearch#56860 Co-authored-by: Russ Cam <russ.cam@elastic.co>
Adds
max_token_length
option to theCharGroupTokenizer
.Updates documentation as well to reflect the changes.
Solves #56676