_analyze api does not correctly use normalizers when specified #48866
Conversation
|
Hi @gaobinlong, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in your Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile? |
|
Hi @elasticcla , sorry, my fault, I have added the e-mail used in the commit into my Github profile. Is there someting else need to do? |
|
Pinging @elastic/es-search (:Search/Analysis) |
There was a problem hiding this comment.
Hi @gaobinlong, great catch and the fix and the test changes look great to me!
I left a short comment and a suggestion around explaining the assumptions in the test better but other than that LGTM. I will wait for you to come back before running some final tests before merging.
server/src/test/java/org/elasticsearch/action/admin/indices/TransportAnalyzeActionTests.java
Show resolved
Hide resolved
Add a comment to explain the result when using custom normalizer
|
@elasticmachine test this please |
|
@elasticmachine update branch |
|
@elasticmachine test this please |
Currently the `_analyze` endpoint doesn't correctly use normalizers specified in the request. This change fixes that by returning the resolved normalizer from TransportAnalyzeAction#getAnalyzer and updates test to be able to catch this in the future. Closes #48650
|
Thanks again @gaobinlong for working on this bug, the PR was much appreciated. I merged it to master and the 7.x branch, so the fix will be included in the upcoming 7.6 release. |
#48650
_analyze api does not correctly use normalizers when specified, because the custom normalizer has never been used. The unit test function-testNormalizerWithIndex() in TransportAnalyzeActionTests class passed because the test case is simple.
This commit fix the bug and use another test case in testNormalizerWithIndex.