addressing comments

Signed-off-by: Sarthak Aggarwal <[email protected]>

server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregator.java

@@ -14,7 +14,7 @@
  *
  * @opensearch.experimental
  */
-public class CountValueAggregator implements ValueAggregator<Long> {
+class CountValueAggregator implements ValueAggregator<Long> {
 
     public static final long DEFAULT_INITIAL_VALUE = 1L;
     private final StarTreeNumericType starTreeNumericType;

server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregator.java

@@ -14,7 +14,7 @@
  *
  * @opensearch.experimental
  */
-public class MaxValueAggregator extends StatelessDoubleValueAggregator {
+class MaxValueAggregator extends StatelessDoubleValueAggregator {
 
     public MaxValueAggregator(StarTreeNumericType starTreeNumericType) {
         super(starTreeNumericType, null);

server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregator.java

@@ -14,7 +14,7 @@
  *
  * @opensearch.experimental
  */
-public class MinValueAggregator extends StatelessDoubleValueAggregator {
+class MinValueAggregator extends StatelessDoubleValueAggregator {
 
     public MinValueAggregator(StarTreeNumericType starTreeNumericType) {
         super(starTreeNumericType, null);

server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StatelessDoubleValueAggregator.java

@@ -15,7 +15,7 @@
  *
  * @opensearch.experimental
  */
-public abstract class StatelessDoubleValueAggregator implements ValueAggregator<Double> {
+abstract class StatelessDoubleValueAggregator implements ValueAggregator<Double> {
 
     protected final StarTreeNumericType starTreeNumericType;
     protected final Double identityValue;
@@ -64,7 +64,7 @@ public Double getIdentityMetricValue() {
     }
 
     /**
-     * Performs min or max aggregation on the value and the segmentDocValue based on the implementation
+     * Performs stateless aggregation on the value and the segmentDocValue based on the implementation
      *
      * @param aggregatedValue aggregated value for the segment so far
      * @param segmentDocValue current segment doc value

server/src/main/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregator.java

@@ -20,12 +20,10 @@
  *
  * @opensearch.experimental
  */
-public class SumValueAggregator implements ValueAggregator<Double> {
+class SumValueAggregator implements ValueAggregator<Double> {
 
     private final StarTreeNumericType starTreeNumericType;
 
-    private double sum = 0;
-    private double compensation = 0;
     private CompensatedSum kahanSummation = new CompensatedSum(0, 0);
 
     public SumValueAggregator(StarTreeNumericType starTreeNumericType) {
@@ -35,13 +33,12 @@ public SumValueAggregator(StarTreeNumericType starTreeNumericType) {
     @Override
     public Double getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue) {
         kahanSummation.reset(0, 0);
+        // add takes care of the sum and compensation internally
         if (segmentDocValue != null) {
             kahanSummation.add(starTreeNumericType.getDoubleValue(segmentDocValue));
         } else {
             kahanSummation.add(getIdentityMetricValue());
         }
-        compensation = kahanSummation.delta();
-        sum = kahanSummation.value();
         return kahanSummation.value();
     }
 
@@ -50,41 +47,36 @@ public Double getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue)
     @Override
     public Double mergeAggregatedValueAndSegmentValue(Double value, Long segmentDocValue) {
         assert value == null || kahanSummation.value() == value;
-        kahanSummation.reset(sum, compensation);
+        // add takes care of the sum and compensation internally
         if (segmentDocValue != null) {
             kahanSummation.add(starTreeNumericType.getDoubleValue(segmentDocValue));
         } else {
             kahanSummation.add(getIdentityMetricValue());
         }
-        compensation = kahanSummation.delta();
-        sum = kahanSummation.value();
         return kahanSummation.value();
     }
 
     @Override
     public Double mergeAggregatedValues(Double value, Double aggregatedValue) {
         assert aggregatedValue == null || kahanSummation.value() == aggregatedValue;
-        kahanSummation.reset(sum, compensation);
+        // add takes care of the sum and compensation internally
         if (value != null) {
             kahanSummation.add(value);
         } else {
             kahanSummation.add(getIdentityMetricValue());
         }
-        compensation = kahanSummation.delta();
-        sum = kahanSummation.value();
         return kahanSummation.value();
     }
 
     @Override
     public Double getInitialAggregatedValue(Double value) {
         kahanSummation.reset(0, 0);
+        // add takes care of the sum and compensation internally
         if (value != null) {
             kahanSummation.add(value);
         } else {
             kahanSummation.add(getIdentityMetricValue());
         }
-        compensation = kahanSummation.delta();
-        sum = kahanSummation.value();
         return kahanSummation.value();
     }
 

server/src/test/java/org/opensearch/index/compositeindex/datacube/startree/aggregators/StaticValueAggregatorTests.java

@@ -10,6 +10,7 @@
 
 import org.apache.lucene.util.NumericUtils;
 import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType;
+import org.opensearch.search.aggregations.metrics.CompensatedSum;
 import org.opensearch.test.OpenSearchTestCase;
 
 public class StaticValueAggregatorTests extends OpenSearchTestCase {
@@ -46,15 +47,11 @@ private static double getAggregatedValue(double[] numbers) {
     }
 
     private double kahanSum(double[] numbers) {
-        double sum = 0.0;
-        double c = 0.0;
+        CompensatedSum compensatedSum = new CompensatedSum(0, 0);
         for (double num : numbers) {
-            double y = num - c;
-            double t = sum + y;
-            c = (t - sum) - y;
-            sum = t;
+            compensatedSum.add(num);
         }
-        return sum;
+        return compensatedSum.value();
     }
 
     private double normalSum(double[] numbers) {