From 24a1d1977e1d64c11cf27f69552aab1720fbb58a Mon Sep 17 00:00:00 2001 From: Vikasht34 Date: Tue, 18 Feb 2025 15:27:05 -0800 Subject: [PATCH] Clean Code Post Lucene 10.0.1 change Signed-off-by: Vikasht34 --- CHANGELOG.md | 1 + .../knn/index/KNNVectorScriptDocValues.java | 29 +++--- .../vectorvalues/KNNVectorValuesIterator.java | 91 +++++++++---------- 3 files changed, 61 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97e814267..fd8da2087 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Maintenance * Update package name to fix compilation issue [#2513](https://github.com/opensearch-project/k-NN/pull/2513) ### Refactoring +* Small Refactor Post Lucene 10.0.1 upgrade [#2541](https://github.com/opensearch-project/k-NN/pull/2541) ## [Unreleased 2.x](https://github.com/opensearch-project/k-NN/compare/2.19...2.x) ### Features diff --git a/src/main/java/org/opensearch/knn/index/KNNVectorScriptDocValues.java b/src/main/java/org/opensearch/knn/index/KNNVectorScriptDocValues.java index b174959c9..08ba7d498 100644 --- a/src/main/java/org/opensearch/knn/index/KNNVectorScriptDocValues.java +++ b/src/main/java/org/opensearch/knn/index/KNNVectorScriptDocValues.java @@ -74,26 +74,33 @@ public float[] get(int i) { /** * Creates a KNNVectorScriptDocValues object based on the provided parameters. * - * @param values The DocIdSetIterator representing the vector values. + * @param knnVectorValues The DocIdSetIterator representing the vector values. * @param fieldName The name of the field. * @param vectorDataType The data type of the vector. * @return A KNNVectorScriptDocValues object based on the type of the values. * @throws IllegalArgumentException If the type of values is unsupported. */ - public static KNNVectorScriptDocValues create(Object values, String fieldName, VectorDataType vectorDataType) { - Objects.requireNonNull(values, "values must not be null"); - - if (values instanceof FloatVectorValues) { - return new KNNFloatVectorScriptDocValues((FloatVectorValues) values, fieldName, vectorDataType); - } else if (values instanceof ByteVectorValues) { - return new KNNByteVectorScriptDocValues((ByteVectorValues) values, fieldName, vectorDataType); - } else if (values instanceof BinaryDocValues) { - return new KNNNativeVectorScriptDocValues((BinaryDocValues) values, fieldName, vectorDataType); + public static KNNVectorScriptDocValues create(KnnVectorValues knnVectorValues, String fieldName, VectorDataType vectorDataType) { + Objects.requireNonNull(knnVectorValues, "values must not be null"); + if (knnVectorValues instanceof FloatVectorValues) { + return new KNNFloatVectorScriptDocValues((FloatVectorValues) knnVectorValues, fieldName, vectorDataType); + } else if (knnVectorValues instanceof ByteVectorValues) { + return new KNNByteVectorScriptDocValues((ByteVectorValues) knnVectorValues, fieldName, vectorDataType); } else { - throw new IllegalArgumentException("Unsupported values type: " + values.getClass()); + throw new IllegalArgumentException("Unsupported values type: " + knnVectorValues.getClass()); } } + public static KNNVectorScriptDocValues create(DocIdSetIterator docIdSetIterator, String fieldName, VectorDataType vectorDataType) { + Objects.requireNonNull(docIdSetIterator, "values must not be null"); + if (docIdSetIterator instanceof BinaryDocValues) { + return new KNNNativeVectorScriptDocValues((BinaryDocValues) docIdSetIterator, fieldName, vectorDataType); + } else { + throw new IllegalArgumentException("Unsupported values type: " + docIdSetIterator.getClass()); + } + + } + private static final class KNNByteVectorScriptDocValues extends KNNVectorScriptDocValues { private final ByteVectorValues values; private final KnnVectorValues.DocIndexIterator iterator; diff --git a/src/main/java/org/opensearch/knn/index/vectorvalues/KNNVectorValuesIterator.java b/src/main/java/org/opensearch/knn/index/vectorvalues/KNNVectorValuesIterator.java index bf9c0bef3..0016fdac6 100644 --- a/src/main/java/org/opensearch/knn/index/vectorvalues/KNNVectorValuesIterator.java +++ b/src/main/java/org/opensearch/knn/index/vectorvalues/KNNVectorValuesIterator.java @@ -67,32 +67,15 @@ public interface KNNVectorValuesIterator { VectorValueExtractorStrategy getVectorExtractorStrategy(); /** - * A DocIdsIteratorValues provides a common iteration logic for all Values that implements - * {@link DocIdSetIterator} interface. Example: {@link BinaryDocValues}, {@link FloatVectorValues} etc. + * Abstract base class for KNN vector iterators, encapsulating common iteration logic. */ - class DocIdsIteratorValues implements KNNVectorValuesIterator { - private final DocIdSetIterator docIdSetIterator; - private KnnVectorValues knnVectorValues = null; // Added reference to KnnVectorValues - @Getter - @Setter - private int lastOrd = -1; - @Getter - @Setter - private Object lastAccessedVector = null; + abstract class AbstractVectorValuesIterator implements KNNVectorValuesIterator { + protected final DocIdSetIterator docIdSetIterator; - DocIdsIteratorValues(@NonNull final KnnVectorValues knnVectorValues) { - this.docIdSetIterator = knnVectorValues.iterator(); - this.knnVectorValues = knnVectorValues; - } - - DocIdsIteratorValues(final DocIdSetIterator docIdSetIterator) { + AbstractVectorValuesIterator(@NonNull final DocIdSetIterator docIdSetIterator) { this.docIdSetIterator = docIdSetIterator; } - public KnnVectorValues getKnnVectorValues() { - return knnVectorValues; - } - @Override public int docId() { return docIdSetIterator.docID(); @@ -113,6 +96,41 @@ public DocIdSetIterator getDocIdSetIterator() { return docIdSetIterator; } + @Override + public long liveDocs() { + return docIdSetIterator.cost(); + } + } + + /** + * A DocIdsIteratorValues provides a common iteration logic for all Values that implements + * {@link DocIdSetIterator} interface. Example: {@link BinaryDocValues}, {@link FloatVectorValues} etc. + */ + class DocIdsIteratorValues extends AbstractVectorValuesIterator { + + private final KnnVectorValues knnVectorValues; + + @Getter + @Setter + private int lastOrd = -1; + @Getter + @Setter + private Object lastAccessedVector = null; + + DocIdsIteratorValues(@NonNull final KnnVectorValues knnVectorValues) { + super(knnVectorValues.iterator()); + this.knnVectorValues = knnVectorValues; + } + + DocIdsIteratorValues(final DocIdSetIterator docIdSetIterator) { + super(docIdSetIterator); + this.knnVectorValues = null; + } + + public KnnVectorValues getKnnVectorValues() { + return knnVectorValues; + } + @Override public long liveDocs() { if (docIdSetIterator instanceof BinaryDocValues) { @@ -129,34 +147,19 @@ public long liveDocs() { public VectorValueExtractorStrategy getVectorExtractorStrategy() { return new VectorValueExtractorStrategy.DISIVectorExtractor(); } + } /** * A FieldWriterIteratorValues is mainly used when Vectors are stored in {@link KnnFieldVectorsWriter} interface. */ - class FieldWriterIteratorValues implements KNNVectorValuesIterator { - private final DocIdSetIterator docIdSetIterator; + class FieldWriterIteratorValues extends AbstractVectorValuesIterator { private final Map vectors; FieldWriterIteratorValues(@NonNull final DocsWithFieldSet docsWithFieldSet, @NonNull final Map vectors) { + super(docsWithFieldSet.iterator()); assert docsWithFieldSet.iterator().cost() == vectors.size(); this.vectors = vectors; - this.docIdSetIterator = docsWithFieldSet.iterator(); - } - - @Override - public int docId() { - return docIdSetIterator.docID(); - } - - @Override - public int advance(int docId) throws IOException { - return docIdSetIterator.advance(docId); - } - - @Override - public int nextDoc() throws IOException { - return docIdSetIterator.nextDoc(); } /** @@ -167,16 +170,6 @@ public T vectorsValue() { return vectors.get(docId()); } - @Override - public DocIdSetIterator getDocIdSetIterator() { - return docIdSetIterator; - } - - @Override - public long liveDocs() { - return docIdSetIterator.cost(); - } - @Override public VectorValueExtractorStrategy getVectorExtractorStrategy() { return new VectorValueExtractorStrategy.FieldWriterIteratorVectorExtractor();