From f31fe9052358a2aacc9361fd4c2f94922510e4bf Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Tue, 20 Aug 2024 17:49:51 +0530 Subject: [PATCH] addressing comments Signed-off-by: Sarthak Aggarwal --- .../Lucene90DocValuesConsumerWrapper.java | 8 +- .../Lucene90DocValuesProducerWrapper.java | 7 +- .../composite/CompositeDocValuesProducer.java | 28 ------ .../LuceneDocValuesConsumerFactory.java | 14 ++- .../LuceneDocValuesProducerFactory.java | 14 ++- .../LuceneDocValuesProducerFactoryTests.java | 7 +- ...tedNumericDocValuesWriterWrapperTests.java | 94 +++++++++++++++++++ 7 files changed, 131 insertions(+), 41 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/index/codec/composite/CompositeDocValuesProducer.java create mode 100644 server/src/test/java/org/opensearch/index/codec/composite/SortedNumericDocValuesWriterWrapperTests.java 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 index 01f139cad8379..67ee45f4c9306 100644 --- 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 index 7708f15d60eda..a213852c59094 100644 --- 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 deleted file mode 100644 index 609c0095b3155..0000000000000 --- a/server/src/main/java/org/opensearch/index/codec/composite/CompositeDocValuesProducer.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * 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.codecs.DocValuesProducer; - -import java.io.Closeable; - -/** - * An interface that provides access to document values for a specific field. - * - * @opensearch.experimental - */ -public interface CompositeDocValuesProducer extends Closeable { - - /** - * Returns the DocValuesProducer instance. - * - * @return The DocValuesProducer instance. - */ - DocValuesProducer getDocValuesProducer(); -} diff --git a/server/src/main/java/org/opensearch/index/codec/composite/LuceneDocValuesConsumerFactory.java b/server/src/main/java/org/opensearch/index/codec/composite/LuceneDocValuesConsumerFactory.java index 0f66dbb31f84b..c68daedc9799e 100644 --- a/server/src/main/java/org/opensearch/index/codec/composite/LuceneDocValuesConsumerFactory.java +++ b/server/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/server/src/main/java/org/opensearch/index/codec/composite/LuceneDocValuesProducerFactory.java b/server/src/main/java/org/opensearch/index/codec/composite/LuceneDocValuesProducerFactory.java index 63827ba32ab71..4362189dbb489 100644 --- a/server/src/main/java/org/opensearch/index/codec/composite/LuceneDocValuesProducerFactory.java +++ b/server/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/server/src/test/java/org/opensearch/index/codec/composite/LuceneDocValuesProducerFactoryTests.java b/server/src/test/java/org/opensearch/index/codec/composite/LuceneDocValuesProducerFactoryTests.java index ab211e6ae8ee3..55d637dfb9cae 100644 --- a/server/src/test/java/org/opensearch/index/codec/composite/LuceneDocValuesProducerFactoryTests.java +++ b/server/src/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/server/src/test/java/org/opensearch/index/codec/composite/SortedNumericDocValuesWriterWrapperTests.java b/server/src/test/java/org/opensearch/index/codec/composite/SortedNumericDocValuesWriterWrapperTests.java new file mode 100644 index 0000000000000..54eead20ef354 --- /dev/null +++ b/server/src/test/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); + } +}