Fixing bug where mapping accepts both dimension and model-id (#2410)

Signed-off-by: Mark Wu <[email protected]>

CHANGELOG.md

@@ -42,6 +42,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 * Fix filter rewrite logic which was resulting in getting inconsistent / incorrect results for cases where filter was getting rewritten for shards (#2359)[https://github.com/opensearch-project/k-NN/pull/2359]
 * 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]
 ### 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)

src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java

@@ -378,6 +378,15 @@ public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserCont
                 );
             }
 
+            // Ensure user-defined dimension and model are mutually exclusive for indicies created after 2.19
+            if (builder.dimension.getValue() != UNSET_MODEL_DIMENSION_IDENTIFIER
+                && builder.modelId.get() != null
+                && parserContext.indexVersionCreated().onOrAfter(Version.V_2_19_0)) {
+                throw new IllegalArgumentException(
+                    String.format(Locale.ROOT, "Cannot specify both a modelId and dimension in the mapping: %s", name)
+                );
+            }
+
             // Check for flat configuration and validate only if index is created after 2.17
             if (isKNNDisabled(parserContext.getSettings()) && parserContext.indexVersionCreated().onOrAfter(Version.V_2_17_0)) {
                 validateFromFlat(builder);

src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperTests.java

@@ -681,7 +681,6 @@ public void testTypeParser_parse_compressionAndModeParameter() {
         XContentBuilder xContentBuilder4 = XContentFactory.jsonBuilder()
             .startObject()
             .field(TYPE_FIELD_NAME, KNN_VECTOR_TYPE)
-            .field(DIMENSION, 10)
             .field(VECTOR_DATA_TYPE_FIELD, VectorDataType.DEFAULT.getValue())
             .field(MODEL_ID, "test")
             .field(MODE_PARAMETER, Mode.ON_DISK.getName())
@@ -892,6 +891,27 @@ public void testTypeParser_parse_fromModel() throws IOException {
         );
 
         assertEquals(modelId, builder.modelId.get());
+
+        // Fails if both model_id and dimension are set after 2.19
+        XContentBuilder xContentBuilder2 = XContentFactory.jsonBuilder()
+            .startObject()
+            .field(TYPE_FIELD_NAME, KNN_VECTOR_TYPE)
+            .field(DIMENSION_FIELD_NAME, TEST_DIMENSION)
+            .field(MODEL_ID, modelId)
+            .endObject();
+
+        expectThrows(
+            IllegalArgumentException.class,
+            () -> typeParser.parse(fieldName, xContentBuilderToMap(xContentBuilder2), buildParserContext(indexName, settings))
+        );
+
+        // Should not fail if both are set before 2.19
+        typeParser.parse(
+            fieldName,
+            xContentBuilderToMap(xContentBuilder2),
+            buildLegacyParserContext(indexName, settings, Version.V_2_18_1)
+        );
+
     }
 
     public void testTypeParser_parse_fromLegacy() throws IOException {