Skip to content

Commit

Permalink
Add version check for full field name validation (#2477)
Browse files Browse the repository at this point in the history
Signed-off-by: Kunal Kotwani <[email protected]>
(cherry picked from commit 811ae2e)
  • Loading branch information
kotwanikunal authored and github-actions[bot] committed Jan 31, 2025
1 parent abae323 commit 0debbdf
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
* Fixing the bug to prevent index.knn setting from being modified or removed on restore snapshot (#2445)[https://github.com/opensearch-project/k-NN/pull/2445]
* Fix Faiss byte vector efficient filter bug (#2448)[https://github.com/opensearch-project/k-NN/pull/2448]
* Fixing bug where mapping accepts both dimension and model-id (#2410)[https://github.com/opensearch-project/k-NN/pull/2410]
* Add version check for full field name validation (#2477)[https://github.com/opensearch-project/k-NN/pull/2477]
### Infrastructure
* Updated C++ version in JNI from c++11 to c++17 [#2259](https://github.com/opensearch-project/k-NN/pull/2259)
* Upgrade bytebuddy and objenesis version to match OpenSearch core and, update github ci runner for macos [#2279](https://github.com/opensearch-project/k-NN/pull/2279)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import static org.opensearch.knn.index.mapper.KNNVectorFieldMapperUtil.createKNNMethodContextFromLegacy;
import static org.opensearch.knn.index.mapper.KNNVectorFieldMapperUtil.createStoredFieldForByteVector;
import static org.opensearch.knn.index.mapper.KNNVectorFieldMapperUtil.createStoredFieldForFloatVector;
import static org.opensearch.knn.index.mapper.KNNVectorFieldMapperUtil.useFullFieldNameValidation;
import static org.opensearch.knn.index.mapper.KNNVectorFieldMapperUtil.validateIfCircuitBreakerIsNotTriggered;
import static org.opensearch.knn.index.mapper.KNNVectorFieldMapperUtil.validateIfKNNPluginEnabled;
import static org.opensearch.knn.index.mapper.ModelFieldMapper.UNSET_MODEL_DIMENSION_IDENTIFIER;
Expand Down Expand Up @@ -240,7 +241,9 @@ protected Explicit<Boolean> ignoreMalformed(BuilderContext context) {

@Override
public KNNVectorFieldMapper build(BuilderContext context) {
validateFullFieldName(context);
if (useFullFieldNameValidation(indexCreatedVersion)) {
validateFullFieldName(context);
}

final MultiFields multiFieldsBuilder = this.multiFieldsBuilder.build(this, context);
final CopyTo copyToBuilder = copyTo.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,16 @@ static boolean useLuceneKNNVectorsFormat(final Version indexCreatedVersion) {
return indexCreatedVersion.onOrAfter(Version.V_2_17_0);
}

/**
* Determines if full field name validation should be applied based on the index creation version.
*
* @param indexCreatedVersion The version when the index was created
* @return true if the index version is 2.17.0 or later, false otherwise
*/
static boolean useFullFieldNameValidation(final Version indexCreatedVersion) {
return indexCreatedVersion != null && indexCreatedVersion.onOrAfter(Version.V_2_17_0);
}

public static SpaceType getSpaceType(final Settings indexSettings) {
String spaceType = indexSettings.get(KNNSettings.INDEX_KNN_SPACE_TYPE.getKey());
if (spaceType == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,7 @@ public void testMethodFieldMapperParseCreateField_validInput_thenDifferentFieldT
when(parseContext.parser()).thenReturn(createXContentParser(dataType));

utilMockedStatic.when(() -> KNNVectorFieldMapperUtil.useLuceneKNNVectorsFormat(Mockito.any())).thenReturn(true);
utilMockedStatic.when(() -> KNNVectorFieldMapperUtil.useFullFieldNameValidation(Mockito.any())).thenReturn(true);

OriginalMappingParameters originalMappingParameters = new OriginalMappingParameters(
dataType,
Expand Down Expand Up @@ -1222,6 +1223,7 @@ public void testMethodFieldMapperParseCreateField_validInput_thenDifferentFieldT
);

utilMockedStatic.when(() -> KNNVectorFieldMapperUtil.useLuceneKNNVectorsFormat(Mockito.any())).thenReturn(false);
utilMockedStatic.when(() -> KNNVectorFieldMapperUtil.useFullFieldNameValidation(Mockito.any())).thenReturn(false);

document = new ParseContext.Document();
contentPath = new ContentPath();
Expand Down Expand Up @@ -1287,6 +1289,7 @@ public void testModelFieldMapperParseCreateField_validInput_thenDifferentFieldTy
when(parseContext.parser()).thenReturn(createXContentParser(dataType));

utilMockedStatic.when(() -> KNNVectorFieldMapperUtil.useLuceneKNNVectorsFormat(Mockito.any())).thenReturn(true);
utilMockedStatic.when(() -> KNNVectorFieldMapperUtil.useFullFieldNameValidation(Mockito.any())).thenReturn(true);

OriginalMappingParameters originalMappingParameters = new OriginalMappingParameters(
VectorDataType.DEFAULT,
Expand Down Expand Up @@ -1335,6 +1338,7 @@ public void testModelFieldMapperParseCreateField_validInput_thenDifferentFieldTy
);

utilMockedStatic.when(() -> KNNVectorFieldMapperUtil.useLuceneKNNVectorsFormat(Mockito.any())).thenReturn(false);
utilMockedStatic.when(() -> KNNVectorFieldMapperUtil.useFullFieldNameValidation(Mockito.any())).thenReturn(true);

document = new ParseContext.Document();
contentPath = new ContentPath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,13 @@ public void testUseLuceneKNNVectorsFormat_withDifferentInputs_thenSuccess() {
Assert.assertFalse(KNNVectorFieldMapperUtil.useLuceneKNNVectorsFormat(Version.V_2_16_0));
Assert.assertTrue(KNNVectorFieldMapperUtil.useLuceneKNNVectorsFormat(Version.V_2_17_0));
}

/**
* Test useFullFieldNameValidation method for different OpenSearch versions
*/
public void testUseFullFieldNameValidation() {
Assert.assertFalse(KNNVectorFieldMapperUtil.useFullFieldNameValidation(Version.V_2_16_0));
Assert.assertTrue(KNNVectorFieldMapperUtil.useFullFieldNameValidation(Version.V_2_17_0));
Assert.assertTrue(KNNVectorFieldMapperUtil.useFullFieldNameValidation(Version.V_2_18_0));
}
}

0 comments on commit 0debbdf

Please sign in to comment.