Address Review Comments

Signed-off-by: Naveen Tatikonda <[email protected]>
opensearch-project · Jun 30, 2023 · 7b6a4e1 · 7b6a4e1
1 parent a588973
commit 7b6a4e1
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 113 deletions.
diff --git a/src/main/java/org/opensearch/knn/index/VectorDataType.java b/src/main/java/org/opensearch/knn/index/VectorDataType.java
@@ -9,15 +9,11 @@
 import org.apache.lucene.document.KnnByteVectorField;
 import org.apache.lucene.document.KnnVectorField;
 import org.apache.lucene.index.DocValuesType;
-import org.apache.lucene.index.IndexOptions;
-import org.apache.lucene.index.IndexableFieldType;
-import org.apache.lucene.index.VectorEncoding;
 import org.apache.lucene.index.VectorSimilarityFunction;
 import org.opensearch.index.mapper.ParametrizedFieldMapper;
 import org.opensearch.knn.index.util.KNNEngine;
 
 import java.util.HashSet;
-import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 
@@ -41,100 +37,6 @@ public enum VectorDataType {
         public FieldType createKnnVectorFieldType(int dimension, VectorSimilarityFunction vectorSimilarityFunction) {
             return KnnByteVectorField.createFieldType(dimension, vectorSimilarityFunction);
         }
-
-        /**
-         * @param knnEngine KNNEngine
-         * @return DocValues FieldType of type Binary and with BYTE VectorEncoding
-         */
-        @Override
-        public FieldType buildDocValuesFieldType(KNNEngine knnEngine) {
-            IndexableFieldType indexableFieldType = new IndexableFieldType() {
-                @Override
-                public boolean stored() {
-                    return false;
-                }
-
-                @Override
-                public boolean tokenized() {
-                    return true;
-                }
-
-                @Override
-                public boolean storeTermVectors() {
-                    return false;
-                }
-
-                @Override
-                public boolean storeTermVectorOffsets() {
-                    return false;
-                }
-
-                @Override
-                public boolean storeTermVectorPositions() {
-                    return false;
-                }
-
-                @Override
-                public boolean storeTermVectorPayloads() {
-                    return false;
-                }
-
-                @Override
-                public boolean omitNorms() {
-                    return false;
-                }
-
-                @Override
-                public IndexOptions indexOptions() {
-                    return IndexOptions.NONE;
-                }
-
-                @Override
-                public DocValuesType docValuesType() {
-                    return DocValuesType.NONE;
-                }
-
-                @Override
-                public int pointDimensionCount() {
-                    return 0;
-                }
-
-                @Override
-                public int pointIndexDimensionCount() {
-                    return 0;
-                }
-
-                @Override
-                public int pointNumBytes() {
-                    return 0;
-                }
-
-                @Override
-                public int vectorDimension() {
-                    return 0;
-                }
-
-                @Override
-                public VectorEncoding vectorEncoding() {
-                    return VectorEncoding.BYTE;
-                }
-
-                @Override
-                public VectorSimilarityFunction vectorSimilarityFunction() {
-                    return VectorSimilarityFunction.EUCLIDEAN;
-                }
-
-                @Override
-                public Map<String, String> getAttributes() {
-                    return null;
-                }
-            };
-            FieldType field = new FieldType(indexableFieldType);
-            field.putAttribute(KNN_ENGINE, knnEngine.getName());
-            field.setDocValuesType(DocValuesType.BINARY);
-            field.freeze();
-            return field;
-        }
     },
     FLOAT("float") {
         /**
@@ -147,19 +49,6 @@ public FieldType createKnnVectorFieldType(int dimension, VectorSimilarityFunctio
             return KnnVectorField.createFieldType(dimension, vectorSimilarityFunction);
         }
 
-        /**
-         * @param knnEngine  KNNEngine
-         * @return  DocValues FieldType of type Binary and with FLOAT32 VectorEncoding
-         */
-        @Override
-        public FieldType buildDocValuesFieldType(KNNEngine knnEngine) {
-            FieldType field = new FieldType();
-            field.putAttribute(KNN_ENGINE, knnEngine.getName());
-            field.setDocValuesType(DocValuesType.BINARY);
-            field.freeze();
-            return field;
-        }
-
     };
 
     private final String value;
@@ -179,7 +68,17 @@ public String getValue() {
 
     public abstract FieldType createKnnVectorFieldType(int dimension, VectorSimilarityFunction vectorSimilarityFunction);
 
-    public abstract FieldType buildDocValuesFieldType(KNNEngine knnEngine);
+    /**
+     * @param knnEngine  KNNEngine
+     * @return  DocValues FieldType of type Binary and with FLOAT32 VectorEncoding
+     */
+    public FieldType buildDocValuesFieldType(KNNEngine knnEngine) {
+        FieldType field = new FieldType();
+        field.putAttribute(KNN_ENGINE, knnEngine.getName());
+        field.setDocValuesType(DocValuesType.BINARY);
+        field.freeze();
+        return field;
+    }
 
     /**
      * @return  Set of names of all the supporting VectorDataTypes

diff --git a/src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperTests.java b/src/test/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapperTests.java
@@ -834,7 +834,6 @@ public void testLuceneFieldMapper_parseCreateField_docValues_withBytes() throws
         }
 
         assertEquals(TEST_BYTE_VECTOR_BYTES_REF, vectorField.binaryValue());
-        assertEquals(VectorEncoding.BYTE, vectorField.fieldType().vectorEncoding());
         assertArrayEquals(TEST_BYTE_VECTOR, knnByteVectorField.vectorValue());
 
         // Test when doc values are disabled