Fix PostingsFormat in KNN Codec #236
Conversation
Codecov Report
@@ Coverage Diff @@
## master #236 +/- ##
=========================================
Coverage ? 78.77%
Complexity ? 339
=========================================
Files ? 53
Lines ? 1343
Branches ? 122
=========================================
Hits ? 1058
Misses ? 232
Partials ? 53
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Could we add test case confirming the fix works? Other than the suggests field, what other functionality does this bug break?
"Elasticsearch initiates PostingsFormat based on the MapperService. For KNNCodec, we do not pass MapperService"
Do you have code link for where Elasticsearch initiates PostingsFormat based on MapperService? Also, why can't we pass MapperService to KNNCodec?
| @@ -82,9 +82,17 @@ public DocValuesFormat docValuesFormat() { | |||
| * approach of manually overriding. | |||
| */ | |||
|
|
|||
| PostingsFormat postingsFormat = null; | |||
| @@ -45,4 +47,8 @@ public Codec codec(String name) { | |||
| } | |||
| return codec; | |||
| } | |||
|
|
|||
| public void setPostingsFormat(PostingsFormat postingsFormat) { | |||
| ((KNN86Codec)codec("")).setPostingsFormat(postingsFormat); | |||
There was a problem hiding this comment.
codec function does not use this String. We could pass any string here. Idea is to get the KNNCodec from this function
|
* postings list fix
Issue #, if available:
#227
Description of changes:
Elasticsearch initiates PostingsFormat based on the MapperService. For KNNCodec, we do not pass MapperService and the PostingsFormat deviates from underlying format for the current Lucene version. Because of this some of the terms that use PostingsFormat could have incompatibility. For example #227.
Updated KNNCodec to rely on the right PostingsFormat used by underlying Codec of Elasticsearch.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.