Refactor KNNVectorFieldMapper #240
Conversation
Codecov Report
@@ Coverage Diff @@
## master #240 +/- ##
=========================================
Coverage ? 78.65%
Complexity ? 336
=========================================
Files ? 53
Lines ? 1326
Branches ? 121
=========================================
Hits ? 1043
Misses ? 234
Partials ? 49
Continue to review full report at Codecov.
|
| fieldType.putAttribute(KNNConstants.SPACE_TYPE, spaceType); | ||
| fieldType.putAttribute(KNNConstants.HNSW_ALGO_M, String.valueOf(m)); | ||
| fieldType.putAttribute(KNNConstants.HNSW_ALGO_EF_CONSTRUCTION, String.valueOf(efConstruction)); |
There was a problem hiding this comment.
How about reading these algo params from the indexSettings here? Idea is to lazy load these settings to ensure the settings are available to read from IndexSettings.
There was a problem hiding this comment.
This would cause index settings to be read everytime a document is indexed. I refactored to build fieldType to the KNNVectorFieldMapper constructor and read the settings from BuilderContext during Builder.build()
| if (spaceType == null) { | ||
| spaceType = getSpaceType(context.indexSettings()); | ||
| } |
There was a problem hiding this comment.
Why are we storing state in Builder class? Why not get a fresh copy every time by reading from indexSettings?
There was a problem hiding this comment.
Makes sense. Updated.
Issue #, if available:
#239
Description of changes:
PR refactors KNNVectorFieldMapper in a variety of ways.
First, it refactors KNNVectorFieldMapper to utilize ParametrizedFieldMapper. This way, we do not need to parse the field parameters manually.
Second, it refactors KNNVectorFieldMapper to build a new fieldType in parseCreateField, as opposed to updating
Defaults.FIELD_TYPEinside the builder logic. This will prevent race condition bugs seen in #239.Third, it refactors the ordering of the logic inside of KNNVectorFieldMapper to follow the convention of other Elasticsearch field mappers.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.