apply review comments

Signed-off-by: panguixin <[email protected]>
opensearch-project · Mar 28, 2024 · b110b5f · b110b5f
1 parent f30c093
commit b110b5f
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 22 deletions.
diff --git a/src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java b/src/main/java/org/opensearch/knn/index/KNNVectorDVLeafFieldData.java
@@ -8,7 +8,6 @@
 import org.apache.lucene.index.DocValues;
 import org.apache.lucene.index.FieldInfo;
 import org.apache.lucene.index.LeafReader;
-import org.apache.lucene.index.VectorEncoding;
 import org.apache.lucene.search.DocIdSetIterator;
 import org.opensearch.index.fielddata.LeafFieldData;
 import org.opensearch.index.fielddata.ScriptDocValues;
@@ -46,11 +45,18 @@ public ScriptDocValues<float[]> getScriptValues() {
                 return KNNVectorScriptDocValues.emptyValues(fieldName, vectorDataType);
             }
 
-            DocIdSetIterator values = null;
+            DocIdSetIterator values;
             if (fieldInfo.hasVectorValues()) {
-                values = fieldInfo.getVectorEncoding() == VectorEncoding.FLOAT32
-                    ? reader.getFloatVectorValues(fieldName)
-                    : reader.getByteVectorValues(fieldName);
+                switch (fieldInfo.getVectorEncoding()) {
+                    case FLOAT32:
+                        values = reader.getFloatVectorValues(fieldName);
+                        break;
+                    case BYTE:
+                        values = reader.getByteVectorValues(fieldName);
+                        break;
+                    default:
+                        throw new IllegalStateException("Unsupported Lucene vector encoding: " + fieldInfo.getVectorEncoding());
+                }
             } else {
                 values = DocValues.getBinary(reader, fieldName);
             }

diff --git a/src/test/java/org/opensearch/knn/plugin/script/KNNScriptScoringIT.java b/src/test/java/org/opensearch/knn/plugin/script/KNNScriptScoringIT.java
@@ -38,23 +38,6 @@
 import static org.hamcrest.Matchers.containsString;
 
 public class KNNScriptScoringIT extends KNNRestTestCase {
-    private void randomCreateKNNIndex() throws IOException {
-        if (randomBoolean()) {
-            createKnnIndex(INDEX_NAME, createKnnIndexMapping(FIELD_NAME, 2));
-        } else {
-            createKnnIndex(
-                INDEX_NAME,
-                createKnnIndexMapping(
-                    FIELD_NAME,
-                    2,
-                    KNNConstants.METHOD_HNSW,
-                    KNNEngine.LUCENE.getName(),
-                    SpaceType.DEFAULT.getValue(),
-                    randomBoolean()
-                )
-            );
-        }
-    }
 
     public void testKNNL2ScriptScore() throws Exception {
         /*
@@ -811,4 +794,26 @@ public void testKNNScriptScoreWithRequestCacheEnabled() throws Exception {
         // assert that the request cache was hit at second request
         assertEquals(1, secondQueryCacheMap.get("hit_count"));
     }
+
+    /**
+     * Create native knn index or Lucene knn index with/without doc values randomly
+     * @throws IOException
+     */
+    private void randomCreateKNNIndex() throws IOException {
+        if (randomBoolean()) {
+            createKnnIndex(INDEX_NAME, createKnnIndexMapping(FIELD_NAME, 2));
+        } else {
+            createKnnIndex(
+                INDEX_NAME,
+                createKnnIndexMapping(
+                    FIELD_NAME,
+                    2,
+                    KNNConstants.METHOD_HNSW,
+                    KNNEngine.LUCENE.getName(),
+                    SpaceType.DEFAULT.getValue(),
+                    randomBoolean()
+                )
+            );
+        }
+    }
 }