Star Tree Merge and Aggregation Fixes

Signed-off-by: Sarthak Aggarwal <[email protected]>
sarthakaggarwal97 · Aug 16, 2024 · 58c8c2a · 58c8c2a
1 parent b316279
commit 58c8c2a
Show file tree

Hide file tree

Showing 11 changed files with 53 additions and 27 deletions.
diff --git a/...g/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregator.java b/...g/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregator.java
@@ -60,7 +60,7 @@ public Long mergeAggregatedValues(Long value, Long aggregatedValue) {
     }
 
     @Override
-    public Long toStarTreeNumericTypeValue(Long value) {
+    public Long toAggregatedValueType(Long value) {
         return value;
     }
 

diff --git a/...ch/index/compositeindex/datacube/startree/aggregators/StatelessDoubleValueAggregator.java b/...ch/index/compositeindex/datacube/startree/aggregators/StatelessDoubleValueAggregator.java
@@ -52,12 +52,12 @@ public Double mergeAggregatedValues(Double value, Double aggregatedValue) {
     }
 
     @Override
-    public Double toStarTreeNumericTypeValue(Long value) {
+    public Double toAggregatedValueType(Long value) {
         try {
             if (value == null) {
                 return getIdentityMetricValue();
             }
-            return starTreeNumericType.getDoubleValue(value);
+            return VALUE_AGGREGATOR_TYPE.getDoubleValue(value);
         } catch (Exception e) {
             throw new IllegalStateException("Cannot convert " + value + " to sortable aggregation type", e);
         }

diff --git a/...org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregator.java b/...org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregator.java
@@ -87,12 +87,12 @@ public Double getInitialAggregatedValue(Double value) {
     }
 
     @Override
-    public Double toStarTreeNumericTypeValue(Long value) {
+    public Double toAggregatedValueType(Long value) {
         try {
             if (value == null) {
                 return getIdentityMetricValue();
             }
-            return starTreeNumericType.getDoubleValue(value);
+            return VALUE_AGGREGATOR_TYPE.getDoubleValue(value);
         } catch (Exception e) {
             throw new IllegalStateException("Cannot convert " + value + " to sortable aggregation type", e);
         }

diff --git a/...va/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregator.java b/...va/org/opensearch/index/compositeindex/datacube/startree/aggregators/ValueAggregator.java
@@ -52,7 +52,7 @@ default A getInitialAggregatedValue(A value) {
     /**
      * Converts an aggregated value from a Long type.
      */
-    A toStarTreeNumericTypeValue(Long rawValue);
+    A toAggregatedValueType(Long rawValue);
 
     /**
      * Fetches a value that does not alter the result of aggregations

diff --git a/...va/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java b/...va/org/opensearch/index/compositeindex/datacube/startree/builder/BaseStarTreeBuilder.java
@@ -164,7 +164,7 @@ protected StarTreeDocument getStarTreeDocument(
             // actual indexing field they're based on
             metrics[i] = metricAggregatorInfos.get(i)
                 .getValueAggregators()
-                .toStarTreeNumericTypeValue(metricDocValuesIterator.value(currentDocId));
+                .toAggregatedValueType(metricDocValuesIterator.value(currentDocId));
             i++;
         }
         return new StarTreeDocument(dims, metrics);

diff --git a/...rc/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeTestUtils.java b/...rc/test/java/org/opensearch/index/compositeindex/datacube/startree/StarTreeTestUtils.java
@@ -0,0 +1,22 @@
+/*
+ * 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.compositeindex.datacube.startree;
+
+import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType;
+
+public class StarTreeTestUtils {
+
+    public static Double toStarTreeNumericTypeValue(Long value, StarTreeNumericType starTreeNumericType) {
+        try {
+            return starTreeNumericType.getDoubleValue(value);
+        } catch (Exception e) {
+            throw new IllegalStateException("Cannot convert " + value + " to sortable aggregation type", e);
+        }
+    }
+}
diff --git a/...arch/index/compositeindex/datacube/startree/aggregators/AbstractValueAggregatorTests.java b/...arch/index/compositeindex/datacube/startree/aggregators/AbstractValueAggregatorTests.java
@@ -10,6 +10,7 @@
 
 import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 
+import org.opensearch.index.compositeindex.datacube.startree.StarTreeTestUtils;
 import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType;
 import org.opensearch.test.OpenSearchTestCase;
 import org.junit.Before;
@@ -21,7 +22,7 @@
 public abstract class AbstractValueAggregatorTests extends OpenSearchTestCase {
 
     private ValueAggregator aggregator;
-    private StarTreeNumericType starTreeNumericType;
+    protected StarTreeNumericType starTreeNumericType;
 
     public AbstractValueAggregatorTests(StarTreeNumericType starTreeNumericType) {
         this.starTreeNumericType = starTreeNumericType;
@@ -69,7 +70,7 @@ public void testGetInitialAggregatedValueForSegmentDocValue() {
             assertEquals(CountValueAggregator.DEFAULT_INITIAL_VALUE, aggregator.getInitialAggregatedValueForSegmentDocValue(randomLong()));
         } else {
             assertEquals(
-                aggregator.toStarTreeNumericTypeValue(randomLong),
+                StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
                 aggregator.getInitialAggregatedValueForSegmentDocValue(randomLong)
             );
         }

diff --git a/...nsearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregatorTests.java b/...nsearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregatorTests.java
@@ -36,10 +36,10 @@ public void testGetInitialAggregatedValue() {
         assertEquals(randomLong, aggregator.getInitialAggregatedValue(randomLong), 0.0);
     }
 
-    public void testToStarTreeNumericTypeValue() {
+    public void testToAggregatedValueType() {
         long randomLong = randomLong();
-        assertEquals(randomLong, aggregator.toStarTreeNumericTypeValue(randomLong), 0.0);
-        assertNull(aggregator.toStarTreeNumericTypeValue(null));
+        assertEquals(randomLong, aggregator.toAggregatedValueType(randomLong), 0.0);
+        assertNull(aggregator.toAggregatedValueType(null));
     }
 
     public void testIdentityMetricValue() {

diff --git a/...pensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregatorTests.java b/...pensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregatorTests.java
@@ -9,6 +9,7 @@
 package org.opensearch.index.compositeindex.datacube.startree.aggregators;
 
 import org.apache.lucene.util.NumericUtils;
+import org.opensearch.index.compositeindex.datacube.startree.StarTreeTestUtils;
 import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType;
 
 public class MaxValueAggregatorTests extends AbstractValueAggregatorTests {
@@ -23,18 +24,18 @@ public void testMergeAggregatedValueAndSegmentValue() {
         Long randomLong = randomLong();
         double randomDouble = randomDouble();
         assertEquals(
-            Math.max(aggregator.toStarTreeNumericTypeValue(randomLong), randomDouble),
+            Math.max(StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType), randomDouble),
             aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, randomLong),
             0.0
         );
         assertEquals(
-            aggregator.toStarTreeNumericTypeValue(randomLong),
+            StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
             aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong),
             0.0
         );
         assertEquals(randomDouble, aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, null), 0.0);
         assertEquals(
-            Math.max(2.0, aggregator.toStarTreeNumericTypeValue(3L)),
+            Math.max(2.0, StarTreeTestUtils.toStarTreeNumericTypeValue(3L, starTreeNumericType)),
             aggregator.mergeAggregatedValueAndSegmentValue(2.0, 3L),
             0.0
         );
@@ -53,10 +54,10 @@ public void testGetInitialAggregatedValue() {
         assertEquals(randomDouble, aggregator.getInitialAggregatedValue(randomDouble), 0.0);
     }
 
-    public void testToStarTreeNumericTypeValue() {
+    public void testToAggregatedValueType() {
         MaxValueAggregator aggregator = new MaxValueAggregator(StarTreeNumericType.DOUBLE);
         long randomLong = randomLong();
-        assertEquals(NumericUtils.sortableLongToDouble(randomLong), aggregator.toStarTreeNumericTypeValue(randomLong), 0.0);
+        assertEquals(NumericUtils.sortableLongToDouble(randomLong), aggregator.toAggregatedValueType(randomLong), 0.0);
     }
 
     public void testIdentityMetricValue() {

diff --git a/...pensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregatorTests.java b/...pensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregatorTests.java
@@ -9,6 +9,7 @@
 package org.opensearch.index.compositeindex.datacube.startree.aggregators;
 
 import org.apache.lucene.util.NumericUtils;
+import org.opensearch.index.compositeindex.datacube.startree.StarTreeTestUtils;
 import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType;
 
 public class MinValueAggregatorTests extends AbstractValueAggregatorTests {
@@ -22,18 +23,18 @@ public void testMergeAggregatedValueAndSegmentValue() {
         Long randomLong = randomLong();
         double randomDouble = randomDouble();
         assertEquals(
-            Math.min(aggregator.toStarTreeNumericTypeValue(randomLong), randomDouble),
+            Math.min(StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType), randomDouble),
             aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, randomLong),
             0.0
         );
         assertEquals(
-            aggregator.toStarTreeNumericTypeValue(randomLong),
+            StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
             aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong),
             0.0
         );
         assertEquals(randomDouble, aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, null), 0.0);
         assertEquals(
-            Math.min(2.0, aggregator.toStarTreeNumericTypeValue(3L)),
+            Math.min(2.0, StarTreeTestUtils.toStarTreeNumericTypeValue(3L, starTreeNumericType)),
             aggregator.mergeAggregatedValueAndSegmentValue(2.0, 3L),
             0.0
         );
@@ -52,10 +53,10 @@ public void testGetInitialAggregatedValue() {
         assertEquals(randomDouble, aggregator.getInitialAggregatedValue(randomDouble), 0.0);
     }
 
-    public void testToStarTreeNumericTypeValue() {
+    public void testToAggregatedValueType() {
         MinValueAggregator aggregator = new MinValueAggregator(StarTreeNumericType.DOUBLE);
         long randomLong = randomLong();
-        assertEquals(NumericUtils.sortableLongToDouble(randomLong), aggregator.toStarTreeNumericTypeValue(randomLong), 0.0);
+        assertEquals(NumericUtils.sortableLongToDouble(randomLong), aggregator.toAggregatedValueType(randomLong), 0.0);
     }
 
     public void testIdentityMetricValue() {

diff --git a/...pensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregatorTests.java b/...pensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregatorTests.java
@@ -8,6 +8,7 @@
 
 package org.opensearch.index.compositeindex.datacube.startree.aggregators;
 
+import org.opensearch.index.compositeindex.datacube.startree.StarTreeTestUtils;
 import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType;
 
 public class SumValueAggregatorTests extends AbstractValueAggregatorTests {
@@ -29,7 +30,7 @@ public void testMergeAggregatedValueAndSegmentValue() {
         Long randomLong = randomLong();
         aggregator.getInitialAggregatedValue(randomDouble);
         assertEquals(
-            randomDouble + aggregator.toStarTreeNumericTypeValue(randomLong),
+            randomDouble + StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
             aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, randomLong),
             0.0
         );
@@ -41,7 +42,7 @@ public void testMergeAggregatedValueAndSegmentValue_nullSegmentDocValue() {
         aggregator.getInitialAggregatedValue(randomDouble1);
         assertEquals(randomDouble1, aggregator.mergeAggregatedValueAndSegmentValue(randomDouble1, null), 0.0);
         assertEquals(
-            randomDouble1 + aggregator.toStarTreeNumericTypeValue(randomLong),
+            randomDouble1 + StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
             aggregator.mergeAggregatedValueAndSegmentValue(randomDouble1, randomLong),
             0.0
         );
@@ -51,7 +52,7 @@ public void testMergeAggregatedValueAndSegmentValue_nullInitialDocValue() {
         Long randomLong = randomLong();
         aggregator.getInitialAggregatedValue(null);
         assertEquals(
-            aggregator.toStarTreeNumericTypeValue(randomLong),
+            StarTreeTestUtils.toStarTreeNumericTypeValue(randomLong, starTreeNumericType),
             aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong),
             0.0
         );
@@ -70,9 +71,9 @@ public void testGetInitialAggregatedValue() {
         assertEquals(randomDouble, aggregator.getInitialAggregatedValue(randomDouble), 0.0);
     }
 
-    public void testToStarTreeNumericTypeValue() {
+    public void testToAggregatedValueType() {
         long randomLong = randomLong();
-        assertEquals(aggregator.toStarTreeNumericTypeValue(randomLong), aggregator.toStarTreeNumericTypeValue(randomLong), 0.0);
+        assertEquals(aggregator.toAggregatedValueType(randomLong), aggregator.toAggregatedValueType(randomLong), 0.0);
     }
 
     public void testIdentityMetricValue() {