Fixed an equality check in StringFieldMapper. #10359
Conversation
The check was ineffective and was causing search_quote_analyzer to be added to the mapping unnecessarily. Closes #10357
|
Hi @jtibshirani thanks a lot for your PR! Would it be possible to have a test along with your fix? Let me know if you need help with that. |
|
I took a stab at a test, thanks! |
| @SuppressWarnings("unchecked") | ||
| Map<String, Object> result = (Map<String, Object>) fieldMap.get(fieldName); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
good test, would be better to move it to a brand new class though, as this existing test class starts a single node cluster but you don't need to send any requests to it. This is a pure unit test, you can call it StringFieldMapperXContentTests and make it extend ElasticsearchTestCase.
There was a problem hiding this comment.
sorry, scratch that, you need parser, which needs the indexService, which needs an index on the cluster. It's all good, leave the test where it is. ;)
javanna
added
:Search/Mapping
|
@rjernst would you mind taking this over? I had a look and the fix seems ok but the test fails when backporting to 1.x, not sure why, I am not too familiar with mappings code. |
|
This change looks good, I will merge later today. |
|
@javanna I'm sure the test fails because 1.x has different behavior than master for specifying analyzers (how analyzer/index_analyzer/search_analyzer relate to each other). I'll get it working on the backport. |
|
thanks @rjernst ! |
|
Just wanted to check the status on this and see if there's anything more I can do! |
|
Thanks @jtibshirani! Sorry it took so long to get around to this. The backport required updating the tests to use |
The check was ineffective and was causing search_quote_analyzer to be added to the mapping unnecessarily. Closes elastic#10357 closes elastic#10359
The check was ineffective and was causing search_quote_analyzer to be added to the mapping unnecessarily. Closes elastic#10357 closes elastic#10359
The check was ineffective and was causing search_quote_analyzer to be added to the mapping unnecessarily.
Closes #10357