From 71074ed17454b7c070260064514a50d4a11eced6 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 6 Feb 2025 14:34:46 -0800 Subject: [PATCH] Add version check for full field name validation (#2477) (#2502) Signed-off-by: Kunal Kotwani (cherry picked from commit 811ae2e6f1cc8ea7c60b7ed106a6da7b638aca27) Co-authored-by: Kunal Kotwani --- CHANGELOG.md | 1 + .../knn/index/mapper/KNNVectorFieldMapper.java | 5 ++++- .../knn/index/mapper/KNNVectorFieldMapperUtil.java | 10 ++++++++++ .../knn/index/mapper/KNNVectorFieldMapperTests.java | 4 ++++ .../index/mapper/KNNVectorFieldMapperUtilTests.java | 9 +++++++++ 5 files changed, 28 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b6e408c85..44986af0cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * Fixing it to retrieve space_type from index setting when both method and top level don't have the value. [#2374](https://github.com/opensearch-project/k-NN/pull/2374) * Fixing the bug where setting rescore as false for on_disk knn_vector query is a no-op (#2399)[https://github.com/opensearch-project/k-NN/pull/2399] * 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) diff --git a/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java b/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java index c2d91c635a..338a913d5f 100644 --- a/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java +++ b/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java @@ -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; @@ -240,7 +241,9 @@ protected Explicit 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(); diff --git a/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperUtil.java b/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperUtil.java index a9ca56d4b5..1240098191 100644 --- a/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperUtil.java +++ b/src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperUtil.java @@ -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) { diff --git a/src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperTests.java b/src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperTests.java index 02125ec5bc..b0c3d2ffe7 100644 --- a/src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperTests.java +++ b/src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperTests.java @@ -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, @@ -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(); @@ -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, @@ -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(); diff --git a/src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperUtilTests.java b/src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperUtilTests.java index d0fa150a55..8bcf9fdbeb 100644 --- a/src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperUtilTests.java +++ b/src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperUtilTests.java @@ -77,4 +77,13 @@ public void testUseLuceneKNNVectorsFormat_withDifferentInputs_thenSuccess() { Assert.assertTrue(KNNVectorFieldMapperUtil.useLuceneKNNVectorsFormat(Version.V_2_17_0)); Assert.assertTrue(KNNVectorFieldMapperUtil.useLuceneKNNVectorsFormat(Version.V_3_0_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)); + } }