nits

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
sarthakaggarwal97 · Jul 9, 2024 · af7dab1 · af7dab1
1 parent 3e94f68
commit af7dab1
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 49 deletions.
diff --git a/server/src/main/java/org/apache/lucene/index/BaseStarTreeBuilder.java b/server/src/main/java/org/apache/lucene/index/BaseStarTreeBuilder.java
@@ -144,16 +144,11 @@ public List<MetricAggregatorInfo> generateMetricAggregatorInfos(MapperService ma
 
                 FieldInfo metricFieldInfos = state.fieldInfos.fieldInfo(metric.getField());
                 DocValuesType metricDocValuesType = metricFieldInfos.getDocValuesType();
-                if (metricType != MetricStat.COUNT) {
-                    // Need not initialize the metric reader with relevant doc id set iterator for COUNT metric type
-                    metricStatReader = starTreeDocValuesIteratorAdapter.getDocValuesIterator(
-                        metricDocValuesType,
-                        metricFieldInfos,
-                        fieldProducerMap.get(metricFieldInfos.name)
-                    );
-                } else {
-                    metricStatReader = new SequentialDocValuesIterator();
-                }
+                metricStatReader = starTreeDocValuesIteratorAdapter.getDocValuesIterator(
+                    metricDocValuesType,
+                    metricFieldInfos,
+                    fieldProducerMap.get(metricFieldInfos.name)
+                );
 
                 MetricAggregatorInfo metricAggregatorInfo = new MetricAggregatorInfo(
                     metricType,
@@ -199,7 +194,7 @@ public List<MetricAggregatorInfo> generateMetricAggregatorInfos(MapperService ma
      * @param dimensionId dimension id
      * @return dimension value
      */
-    public abstract long getDimensionValue(int docId, int dimensionId) throws IOException;
+    public abstract Long getDimensionValue(int docId, int dimensionId) throws IOException;
 
     /**
      * Sorts and aggregates the star-tree document in the segment, and returns a star-tree document iterator for all the
@@ -522,10 +517,10 @@ private Map<Long, StarTreeBuilderUtils.TreeNode> constructNonStarNodes(int start
         throws IOException {
         Map<Long, StarTreeBuilderUtils.TreeNode> nodes = new HashMap<>();
         int nodeStartDocId = startDocId;
-        long nodeDimensionValue = getDimensionValue(startDocId, dimensionId);
+        Long nodeDimensionValue = getDimensionValue(startDocId, dimensionId);
         for (int i = startDocId + 1; i < endDocId; i++) {
-            long dimensionValue = getDimensionValue(i, dimensionId);
-            if (dimensionValue != nodeDimensionValue) {
+            Long dimensionValue = getDimensionValue(i, dimensionId);
+            if (!dimensionValue.equals(nodeDimensionValue)) {
                 StarTreeBuilderUtils.TreeNode child = getNewNode();
                 child.dimensionId = dimensionId;
                 child.dimensionValue = nodeDimensionValue;

diff --git a/.../org/opensearch/index/compositeindex/datacube/startree/builder/OnHeapStarTreeBuilder.java b/.../org/opensearch/index/compositeindex/datacube/startree/builder/OnHeapStarTreeBuilder.java
@@ -21,6 +21,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 /**
  * On heap single tree builder
@@ -64,9 +65,8 @@ public List<StarTreeDocument> getStarTreeDocuments() {
         return starTreeDocuments;
     }
 
-    // TODO: should this be just long?
     @Override
-    public long getDimensionValue(int docId, int dimensionId) throws IOException {
+    public Long getDimensionValue(int docId, int dimensionId) throws IOException {
         return starTreeDocuments.get(docId).dimensions[dimensionId];
     }
 
@@ -90,7 +90,7 @@ public Iterator<StarTreeDocument> sortAndAggregateStarTreeDocuments(StarTreeDocu
         // sort the documents
         Arrays.sort(starTreeDocuments, (o1, o2) -> {
             for (int i = 0; i < numDimensions; i++) {
-                if (o1.dimensions[i] != o2.dimensions[i]) {
+                if (!Objects.equals(o1.dimensions[i], o2.dimensions[i])) {
                     return Long.compare(o1.dimensions[i], o2.dimensions[i]);
                 }
             }
@@ -154,7 +154,7 @@ public Iterator<StarTreeDocument> generateStarTreeDocumentsForStarNode(int start
         }
         Arrays.sort(starTreeDocuments, (o1, o2) -> {
             for (int i = dimensionId + 1; i < numDimensions; i++) {
-                if (o1.dimensions[i] != o2.dimensions[i]) {
+                if (!Objects.equals(o1.dimensions[i], o2.dimensions[i])) {
                     return Long.compare(o1.dimensions[i], o2.dimensions[i]);
                 }
             }
@@ -167,7 +167,7 @@ public Iterator<StarTreeDocument> generateStarTreeDocumentsForStarNode(int start
 
             private boolean hasSameDimensions(StarTreeDocument starTreeDocument1, StarTreeDocument starTreeDocument2) {
                 for (int i = dimensionId + 1; i < numDimensions; i++) {
-                    if (starTreeDocument1.dimensions[i] != starTreeDocument2.dimensions[i]) {
+                    if (!Objects.equals(starTreeDocument1.dimensions[i], starTreeDocument2.dimensions[i])) {
                         return false;
                     }
                 }

diff --git a/.../opensearch/index/compositeindex/datacube/startree/utils/SequentialDocValuesIterator.java b/.../opensearch/index/compositeindex/datacube/startree/utils/SequentialDocValuesIterator.java
@@ -44,34 +44,6 @@ public SequentialDocValuesIterator(DocIdSetIterator docIdSetIterator) {
         this.docIdSetIterator = docIdSetIterator;
     }
 
-    /**
-     * Creates a SequentialDocValuesIterator with an empty DocIdSetIterator.
-     *
-     */
-    public SequentialDocValuesIterator() {
-        this.docIdSetIterator = new DocIdSetIterator() {
-            @Override
-            public int docID() {
-                return NO_MORE_DOCS;
-            }
-
-            @Override
-            public int nextDoc() {
-                return NO_MORE_DOCS;
-            }
-
-            @Override
-            public int advance(int target) {
-                return NO_MORE_DOCS;
-            }
-
-            @Override
-            public long cost() {
-                return 0;
-            }
-        };
-    }
-
     /**
      * Returns the value associated with the latest document.
      *

diff --git a/...g/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilderTests.java b/...g/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilderTests.java
@@ -171,8 +171,8 @@ public List<StarTreeDocument> getStarTreeDocuments() {
             }
 
             @Override
-            public long getDimensionValue(int docId, int dimensionId) throws IOException {
-                return 0;
+            public Long getDimensionValue(int docId, int dimensionId) throws IOException {
+                return 0L;
             }
 
             @Override