addressing comments

Signed-off-by: Sarthak Aggarwal <[email protected]>
opensearch-project · Aug 20, 2024 · 23bc749 · 23bc749
1 parent 730e4ec
commit 23bc749
Show file tree

Hide file tree

Showing 7 changed files with 131 additions and 41 deletions.
diff --git a/server/src/main/java/org/apache/lucene/codecs/lucene90/Lucene90DocValuesConsumerWrapper.java b/server/src/main/java/org/apache/lucene/codecs/lucene90/Lucene90DocValuesConsumerWrapper.java
@@ -11,6 +11,7 @@
 import org.apache.lucene.codecs.DocValuesConsumer;
 import org.apache.lucene.index.SegmentWriteState;
 
+import java.io.Closeable;
 import java.io.IOException;
 
 /**
@@ -20,7 +21,7 @@
  *
  * @opensearch.experimental
  */
-public class Lucene90DocValuesConsumerWrapper {
+public class Lucene90DocValuesConsumerWrapper implements Closeable {
 
     private final Lucene90DocValuesConsumer lucene90DocValuesConsumer;
 
@@ -37,4 +38,9 @@ public Lucene90DocValuesConsumerWrapper(
     public Lucene90DocValuesConsumer getLucene90DocValuesConsumer() {
         return lucene90DocValuesConsumer;
     }
+
+    @Override
+    public void close() throws IOException {
+        lucene90DocValuesConsumer.close();
+    }
 }
diff --git a/server/src/main/java/org/apache/lucene/codecs/lucene90/Lucene90DocValuesProducerWrapper.java b/server/src/main/java/org/apache/lucene/codecs/lucene90/Lucene90DocValuesProducerWrapper.java
@@ -10,8 +10,8 @@
 
 import org.apache.lucene.codecs.DocValuesProducer;
 import org.apache.lucene.index.SegmentReadState;
-import org.opensearch.index.codec.composite.CompositeDocValuesProducer;
 
+import java.io.Closeable;
 import java.io.IOException;
 
 /**
@@ -21,7 +21,7 @@
  *
  * @opensearch.experimental
  */
-public class Lucene90DocValuesProducerWrapper implements CompositeDocValuesProducer {
+public class Lucene90DocValuesProducerWrapper implements Closeable {
 
     private final Lucene90DocValuesProducer lucene90DocValuesProducer;
 
@@ -35,8 +35,7 @@ public Lucene90DocValuesProducerWrapper(
         lucene90DocValuesProducer = new Lucene90DocValuesProducer(state, dataCodec, dataExtension, metaCodec, metaExtension);
     }
 
-    @Override
-    public DocValuesProducer getDocValuesProducer() {
+    public DocValuesProducer getLucene90DocValuesProducer() {
         return lucene90DocValuesProducer;
     }
 

diff --git a/server/src/main/java/org/opensearch/index/codec/composite/CompositeDocValuesProducer.java b/server/src/main/java/org/opensearch/index/codec/composite/CompositeDocValuesProducer.java
diff --git a/...er/src/main/java/org/opensearch/index/codec/composite/LuceneDocValuesConsumerFactory.java b/...er/src/main/java/org/opensearch/index/codec/composite/LuceneDocValuesConsumerFactory.java
@@ -29,9 +29,17 @@ public static DocValuesConsumer getDocValuesConsumerForCompositeCodec(
         String metaCodec,
         String metaExtension
     ) throws IOException {
-
-        return new Lucene90DocValuesConsumerWrapper(state, dataCodec, dataExtension, metaCodec, metaExtension)
-            .getLucene90DocValuesConsumer();
+        try (
+            Lucene90DocValuesConsumerWrapper lucene90DocValuesConsumerWrapper = new Lucene90DocValuesConsumerWrapper(
+                state,
+                dataCodec,
+                dataExtension,
+                metaCodec,
+                metaExtension
+            )
+        ) {
+            return lucene90DocValuesConsumerWrapper.getLucene90DocValuesConsumer();
+        }
     }
 
 }
diff --git a/...er/src/main/java/org/opensearch/index/codec/composite/LuceneDocValuesProducerFactory.java b/...er/src/main/java/org/opensearch/index/codec/composite/LuceneDocValuesProducerFactory.java
@@ -23,7 +23,7 @@
  */
 public class LuceneDocValuesProducerFactory {
 
-    public static CompositeDocValuesProducer getDocValuesProducerForCompositeCodec(
+    public static DocValuesProducer getDocValuesProducerForCompositeCodec(
         String compositeCodec,
         SegmentReadState state,
         String dataCodec,
@@ -34,7 +34,17 @@ public static CompositeDocValuesProducer getDocValuesProducerForCompositeCodec(
 
         switch (compositeCodec) {
             case Composite99Codec.COMPOSITE_INDEX_CODEC_NAME:
-                return new Lucene90DocValuesProducerWrapper(state, dataCodec, dataExtension, metaCodec, metaExtension);
+                try (
+                    Lucene90DocValuesProducerWrapper lucene90DocValuesProducerWrapper = new Lucene90DocValuesProducerWrapper(
+                        state,
+                        dataCodec,
+                        dataExtension,
+                        metaCodec,
+                        metaExtension
+                    )
+                ) {
+                    return lucene90DocValuesProducerWrapper.getLucene90DocValuesProducer();
+                }
             default:
                 throw new IllegalStateException("Invalid composite codec " + "[" + compositeCodec + "]");
         }

diff --git a/...c/test/java/org/opensearch/index/codec/composite/LuceneDocValuesProducerFactoryTests.java b/...c/test/java/org/opensearch/index/codec/composite/LuceneDocValuesProducerFactoryTests.java
@@ -9,6 +9,7 @@
 package org.opensearch.index.codec.composite;
 
 import org.apache.lucene.codecs.DocValuesConsumer;
+import org.apache.lucene.codecs.DocValuesProducer;
 import org.apache.lucene.codecs.lucene99.Lucene99Codec;
 import org.apache.lucene.index.FieldInfo;
 import org.apache.lucene.index.FieldInfos;
@@ -83,7 +84,7 @@ public void testGetDocValuesProducerForCompositeCodec99() throws IOException {
             new FieldInfos(new FieldInfo[0]),
             newIOContext(random())
         );
-        CompositeDocValuesProducer producer = LuceneDocValuesProducerFactory.getDocValuesProducerForCompositeCodec(
+        DocValuesProducer producer = LuceneDocValuesProducerFactory.getDocValuesProducerForCompositeCodec(
             Composite99Codec.COMPOSITE_INDEX_CODEC_NAME,
             segmentReadState,
             dataCodec,
@@ -93,8 +94,8 @@ public void testGetDocValuesProducerForCompositeCodec99() throws IOException {
         );
 
         assertNotNull(producer);
-        assertEquals("org.apache.lucene.codecs.lucene90.Lucene90DocValuesProducer", producer.getDocValuesProducer().getClass().getName());
-        producer.getDocValuesProducer().close();
+        assertEquals("org.apache.lucene.codecs.lucene90.Lucene90DocValuesProducer", producer.getClass().getName());
+        producer.close();
     }
 
     public void testGetDocValuesProducerForCompositeCodec_InvalidCodec() {

diff --git a/...t/java/org/opensearch/index/codec/composite/SortedNumericDocValuesWriterWrapperTests.java b/...t/java/org/opensearch/index/codec/composite/SortedNumericDocValuesWriterWrapperTests.java
@@ -0,0 +1,94 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.index.codec.composite;
+
+import org.apache.lucene.index.DocValuesType;
+import org.apache.lucene.index.FieldInfo;
+import org.apache.lucene.index.IndexOptions;
+import org.apache.lucene.index.SortedNumericDocValues;
+import org.apache.lucene.index.SortedNumericDocValuesWriterWrapper;
+import org.apache.lucene.index.VectorEncoding;
+import org.apache.lucene.index.VectorSimilarityFunction;
+import org.apache.lucene.util.Counter;
+import org.opensearch.test.OpenSearchTestCase;
+
+import java.io.IOException;
+import java.util.Collections;
+
+public class SortedNumericDocValuesWriterWrapperTests extends OpenSearchTestCase {
+
+    private SortedNumericDocValuesWriterWrapper wrapper;
+    private FieldInfo fieldInfo;
+    private Counter counter;
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp();
+        fieldInfo = new FieldInfo(
+            "field",
+            1,
+            false,
+            false,
+            true,
+            IndexOptions.NONE,
+            DocValuesType.NONE,
+            -1,
+            Collections.emptyMap(),
+            0,
+            0,
+            0,
+            0,
+            VectorEncoding.FLOAT32,
+            VectorSimilarityFunction.EUCLIDEAN,
+            false,
+            false
+        );
+        counter = Counter.newCounter();
+        wrapper = new SortedNumericDocValuesWriterWrapper(fieldInfo, counter);
+    }
+
+    public void testAddValue() throws IOException {
+        wrapper.addValue(0, 10);
+        wrapper.addValue(1, 20);
+        wrapper.addValue(2, 30);
+
+        SortedNumericDocValues docValues = wrapper.getDocValues();
+        assertNotNull(docValues);
+
+        assertEquals(0, docValues.nextDoc());
+        assertEquals(10, docValues.nextValue());
+        assertEquals(1, docValues.nextDoc());
+        assertEquals(20, docValues.nextValue());
+        assertEquals(2, docValues.nextDoc());
+        assertEquals(30, docValues.nextValue());
+    }
+
+    public void testGetDocValues() {
+        SortedNumericDocValues docValues = wrapper.getDocValues();
+        assertNotNull(docValues);
+    }
+
+    public void testMultipleValues() throws IOException {
+        wrapper.addValue(0, 10);
+        wrapper.addValue(0, 20);
+        wrapper.addValue(1, 30);
+
+        SortedNumericDocValues docValues = wrapper.getDocValues();
+        assertNotNull(docValues);
+
+        assertEquals(0, docValues.nextDoc());
+        assertEquals(10, docValues.nextValue());
+        assertEquals(20, docValues.nextValue());
+        assertThrows(IllegalStateException.class, docValues::nextValue);
+
+        assertEquals(1, docValues.nextDoc());
+        assertEquals(30, docValues.nextValue());
+        assertThrows(IllegalStateException.class, docValues::nextValue);
+    }
+}