added tests

Signed-off-by: Sarthak Aggarwal <[email protected]>
bharath-techie · Jul 28, 2024 · dd3a350 · dd3a350
1 parent 51a4a3f
commit dd3a350
Show file tree

Hide file tree

Showing 10 changed files with 529 additions and 202 deletions.
diff --git a/...g/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregator.java b/...g/opensearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregator.java
@@ -36,34 +36,38 @@ public StarTreeNumericType getAggregatedValueType() {
 
     @Override
     public Long getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue) {
+
+        if (segmentDocValue == null) {
+            return getIdentityMetricValue();
+        }
+
         return DEFAULT_INITIAL_VALUE;
     }
 
     @Override
     public Long mergeAggregatedValueAndSegmentValue(Long value, Long segmentDocValue) {
-        long totalCount = getIdentityMetricValue();
-        if (value != null) {
-            totalCount += value;
+        if (value == null) {
+            return getIdentityMetricValue();
         }
-        return totalCount + 1;
+        return value + 1;
     }
 
     @Override
     public Long mergeAggregatedValues(Long value, Long aggregatedValue) {
-        long totalCount = getIdentityMetricValue();
-        if (value != null) {
-            totalCount += value;
+        if (value == null) {
+            value = getIdentityMetricValue();
         }
-
-        if (aggregatedValue != null) {
-            totalCount += aggregatedValue;
+        if (aggregatedValue == null) {
+            aggregatedValue = getIdentityMetricValue();
         }
-
-        return totalCount;
+        return value + aggregatedValue;
     }
 
     @Override
     public Long getInitialAggregatedValue(Long value) {
+        if (value == null) {
+            return getIdentityMetricValue();
+        }
         return value;
     }
 

diff --git a/...org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregator.java b/...org/opensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregator.java
@@ -47,6 +47,8 @@ public Double getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue)
     public Double mergeAggregatedValueAndSegmentValue(Double value, Long segmentDocValue) {
         if (segmentDocValue == null && value != null) {
             return value;
+        } else if (segmentDocValue != null && value == null) {
+            return starTreeNumericType.getDoubleValue(segmentDocValue);
         } else if (segmentDocValue == null) {
             return getIdentityMetricValue();
         }
@@ -57,6 +59,8 @@ public Double mergeAggregatedValueAndSegmentValue(Double value, Long segmentDocV
     public Double mergeAggregatedValues(Double value, Double aggregatedValue) {
         if (value == null && aggregatedValue != null) {
             return aggregatedValue;
+        } else if (value != null && aggregatedValue == null) {
+            return value;
         } else if (value == null) {
             return getIdentityMetricValue();
         }

diff --git a/...org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregator.java b/...org/opensearch/index/compositeindex/datacube/startree/aggregators/MinValueAggregator.java
@@ -47,6 +47,8 @@ public Double getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue)
     public Double mergeAggregatedValueAndSegmentValue(Double value, Long segmentDocValue) {
         if (segmentDocValue == null && value != null) {
             return value;
+        } else if (segmentDocValue != null && value == null) {
+            return starTreeNumericType.getDoubleValue(segmentDocValue);
         } else if (segmentDocValue == null) {
             return getIdentityMetricValue();
         }
@@ -57,6 +59,8 @@ public Double mergeAggregatedValueAndSegmentValue(Double value, Long segmentDocV
     public Double mergeAggregatedValues(Double value, Double aggregatedValue) {
         if (value == null && aggregatedValue != null) {
             return aggregatedValue;
+        } else if (value != null && aggregatedValue == null) {
+            return value;
         } else if (value == null) {
             return getIdentityMetricValue();
         }

diff --git a/...org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregator.java b/...org/opensearch/index/compositeindex/datacube/startree/aggregators/SumValueAggregator.java
@@ -45,6 +45,8 @@ public Double getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue)
         kahanSummation.reset(0, 0);
         if (segmentDocValue != null) {
             kahanSummation.add(starTreeNumericType.getDoubleValue(segmentDocValue));
+        } else {
+            kahanSummation.add(getIdentityMetricValue());
         }
         compensation = kahanSummation.delta();
         sum = kahanSummation.value();
@@ -53,10 +55,12 @@ public Double getInitialAggregatedValueForSegmentDocValue(Long segmentDocValue)
 
     @Override
     public Double mergeAggregatedValueAndSegmentValue(Double value, Long segmentDocValue) {
-        assert kahanSummation.value() == value;
+        assert value == null || kahanSummation.value() == value;
         kahanSummation.reset(sum, compensation);
         if (segmentDocValue != null) {
             kahanSummation.add(starTreeNumericType.getDoubleValue(segmentDocValue));
+        } else {
+            kahanSummation.add(getIdentityMetricValue());
         }
         compensation = kahanSummation.delta();
         sum = kahanSummation.value();
@@ -65,10 +69,12 @@ public Double mergeAggregatedValueAndSegmentValue(Double value, Long segmentDocV
 
     @Override
     public Double mergeAggregatedValues(Double value, Double aggregatedValue) {
-        assert kahanSummation.value() == aggregatedValue;
+        assert aggregatedValue == null || kahanSummation.value() == aggregatedValue;
         kahanSummation.reset(sum, compensation);
         if (value != null) {
             kahanSummation.add(value);
+        } else {
+            kahanSummation.add(getIdentityMetricValue());
         }
         compensation = kahanSummation.delta();
         sum = kahanSummation.value();
@@ -80,6 +86,8 @@ public Double getInitialAggregatedValue(Double value) {
         kahanSummation.reset(0, 0);
         if (value != null) {
             kahanSummation.add(value);
+        } else {
+            kahanSummation.add(getIdentityMetricValue());
         }
         compensation = kahanSummation.delta();
         sum = kahanSummation.value();

diff --git a/...arch/index/compositeindex/datacube/startree/aggregators/AbstractValueAggregatorTests.java b/...arch/index/compositeindex/datacube/startree/aggregators/AbstractValueAggregatorTests.java
@@ -0,0 +1,63 @@
+/*
+ * 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.aggregators;
+
+import org.opensearch.index.compositeindex.datacube.MetricStat;
+import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType;
+import org.opensearch.test.OpenSearchTestCase;
+import org.junit.Before;
+
+public abstract class AbstractValueAggregatorTests extends OpenSearchTestCase {
+
+    private ValueAggregator aggregator;
+
+    @Before
+    public void setup() {
+        aggregator = getValueAggregator();
+    }
+
+    public abstract ValueAggregator getValueAggregator();
+
+    public abstract MetricStat getMetricStat();
+
+    public abstract StarTreeNumericType getValueAggregatorType();
+
+    public void testGetAggregationType() {
+        assertEquals(getMetricStat().getTypeName(), aggregator.getAggregationType().getTypeName());
+    }
+
+    public void testGetAggregatedValueType() {
+        assertEquals(getValueAggregatorType(), aggregator.getAggregatedValueType());
+    }
+
+    public void testGetInitialAggregatedValueForSegmentDocNullValue() {
+        assertEquals(aggregator.getIdentityMetricValue(), aggregator.getInitialAggregatedValueForSegmentDocValue(null));
+    }
+
+    public void testMergeAggregatedNullValueAndSegmentNullValue() {
+        assertEquals(aggregator.getIdentityMetricValue(), aggregator.mergeAggregatedValueAndSegmentValue(null, null));
+    }
+
+    public void testMergeAggregatedNullValues() {
+        assertEquals(aggregator.getIdentityMetricValue(), aggregator.mergeAggregatedValues(null, null));
+    }
+
+    public void testGetInitialAggregatedNullValue() {
+        assertEquals(aggregator.getIdentityMetricValue(), aggregator.getInitialAggregatedValue(null));
+    }
+
+    public void testGetInitialAggregatedValueForSegmentDocValue() {
+        Long randomLong = randomLong();
+        if (aggregator instanceof CountValueAggregator) {
+            assertEquals(CountValueAggregator.DEFAULT_INITIAL_VALUE, aggregator.getInitialAggregatedValueForSegmentDocValue(randomLong()));
+        } else {
+            assertEquals(randomLong.doubleValue(), aggregator.getInitialAggregatedValueForSegmentDocValue(randomLong));
+        }
+    }
+}
diff --git a/...nsearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregatorTests.java b/...nsearch/index/compositeindex/datacube/startree/aggregators/CountValueAggregatorTests.java
@@ -10,48 +10,61 @@
 
 import org.opensearch.index.compositeindex.datacube.MetricStat;
 import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType;
-import org.opensearch.test.OpenSearchTestCase;
 
-public class CountValueAggregatorTests extends OpenSearchTestCase {
-    private final CountValueAggregator aggregator = new CountValueAggregator(StarTreeNumericType.LONG);
-
-    public void testGetAggregationType() {
-        assertEquals(MetricStat.COUNT.getTypeName(), aggregator.getAggregationType().getTypeName());
-    }
+public class CountValueAggregatorTests extends AbstractValueAggregatorTests {
 
-    public void testGetAggregatedValueType() {
-        assertEquals(CountValueAggregator.VALUE_AGGREGATOR_TYPE, aggregator.getAggregatedValueType());
-    }
-
-    public void testGetInitialAggregatedValueForSegmentDocValue() {
-        assertEquals(1L, aggregator.getInitialAggregatedValueForSegmentDocValue(randomLong()), 0.0);
-    }
+    private final CountValueAggregator aggregator = new CountValueAggregator(StarTreeNumericType.LONG);
 
     public void testMergeAggregatedValueAndSegmentValue() {
-        assertEquals(3L, aggregator.mergeAggregatedValueAndSegmentValue(2L, 3L), 0.0);
+        long randomLong = randomLong();
+        assertEquals(randomLong + 1, aggregator.mergeAggregatedValueAndSegmentValue(randomLong, 3L), 0.0);
     }
 
     public void testMergeAggregatedValues() {
-        assertEquals(5L, aggregator.mergeAggregatedValues(2L, 3L), 0.0);
+        long randomLong1 = randomLong();
+        long randomLong2 = randomLong();
+        assertEquals(randomLong1 + randomLong2, aggregator.mergeAggregatedValues(randomLong1, randomLong2), 0.0);
+        assertEquals(randomLong1, aggregator.mergeAggregatedValues(randomLong1, null), 0.0);
+        assertEquals(randomLong2, aggregator.mergeAggregatedValues(null, randomLong2), 0.0);
     }
 
     public void testGetInitialAggregatedValue() {
-        assertEquals(3L, aggregator.getInitialAggregatedValue(3L), 0.0);
+        long randomLong = randomLong();
+        assertEquals(randomLong, aggregator.getInitialAggregatedValue(randomLong), 0.0);
     }
 
     public void testGetMaxAggregatedValueByteSize() {
         assertEquals(Long.BYTES, aggregator.getMaxAggregatedValueByteSize());
     }
 
     public void testToLongValue() {
-        assertEquals(3L, aggregator.toLongValue(3L), 0.0);
+        long randomLong = randomLong();
+        assertEquals(randomLong, aggregator.toLongValue(randomLong), 0.0);
+        assertNull(aggregator.toLongValue(null));
     }
 
     public void testToStarTreeNumericTypeValue() {
-        assertEquals(3L, aggregator.toStarTreeNumericTypeValue(3L), 0.0);
+        long randomLong = randomLong();
+        assertEquals(randomLong, aggregator.toStarTreeNumericTypeValue(randomLong), 0.0);
+        assertNull(aggregator.toStarTreeNumericTypeValue(null));
     }
 
     public void testIdentityMetricValue() {
         assertEquals(0L, aggregator.getIdentityMetricValue(), 0);
     }
+
+    @Override
+    public ValueAggregator getValueAggregator() {
+        return aggregator;
+    }
+
+    @Override
+    public MetricStat getMetricStat() {
+        return MetricStat.COUNT;
+    }
+
+    @Override
+    public StarTreeNumericType getValueAggregatorType() {
+        return StarTreeNumericType.LONG;
+    }
 }
diff --git a/...pensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregatorTests.java b/...pensearch/index/compositeindex/datacube/startree/aggregators/MaxValueAggregatorTests.java
@@ -11,50 +11,64 @@
 import org.apache.lucene.util.NumericUtils;
 import org.opensearch.index.compositeindex.datacube.MetricStat;
 import org.opensearch.index.compositeindex.datacube.startree.aggregators.numerictype.StarTreeNumericType;
-import org.opensearch.test.OpenSearchTestCase;
 
-public class MaxValueAggregatorTests extends OpenSearchTestCase {
-    private final MaxValueAggregator aggregator = new MaxValueAggregator(StarTreeNumericType.LONG);
-
-    public void testGetAggregationType() {
-        assertEquals(MetricStat.MAX.getTypeName(), aggregator.getAggregationType().getTypeName());
-    }
+public class MaxValueAggregatorTests extends AbstractValueAggregatorTests {
 
-    public void testGetAggregatedValueType() {
-        assertEquals(MaxValueAggregator.VALUE_AGGREGATOR_TYPE, aggregator.getAggregatedValueType());
-    }
-
-    public void testGetInitialAggregatedValueForSegmentDocValue() {
-        assertEquals(1.0, aggregator.getInitialAggregatedValueForSegmentDocValue(1L), 0.0);
-        assertNull(aggregator.getInitialAggregatedValueForSegmentDocValue(null));
-    }
+    private final MaxValueAggregator aggregator = new MaxValueAggregator(StarTreeNumericType.LONG);
 
     public void testMergeAggregatedValueAndSegmentValue() {
+        Long randomLong = randomNonNegativeLong();
+        double randomDouble = randomDouble();
+        assertEquals(randomLong.doubleValue(), aggregator.mergeAggregatedValueAndSegmentValue(Double.MIN_VALUE, randomLong), 0.0);
+        assertEquals(randomLong.doubleValue(), aggregator.mergeAggregatedValueAndSegmentValue(null, randomLong), 0.0);
+        assertEquals(randomDouble, aggregator.mergeAggregatedValueAndSegmentValue(randomDouble, null), 0.0);
         assertEquals(3.0, aggregator.mergeAggregatedValueAndSegmentValue(2.0, 3L), 0.0);
     }
 
     public void testMergeAggregatedValues() {
+        double randomDouble = randomDouble();
+        assertEquals(randomDouble, aggregator.mergeAggregatedValues(Double.MIN_VALUE, randomDouble), 0.0);
+        assertEquals(randomDouble, aggregator.mergeAggregatedValues(null, randomDouble), 0.0);
+        assertEquals(randomDouble, aggregator.mergeAggregatedValues(randomDouble, null), 0.0);
         assertEquals(3.0, aggregator.mergeAggregatedValues(2.0, 3.0), 0.0);
     }
 
     public void testGetInitialAggregatedValue() {
-        assertEquals(3.0, aggregator.getInitialAggregatedValue(3.0), 0.0);
+        double randomDouble = randomDouble();
+        assertEquals(randomDouble, aggregator.getInitialAggregatedValue(randomDouble), 0.0);
     }
 
     public void testGetMaxAggregatedValueByteSize() {
         assertEquals(Double.BYTES, aggregator.getMaxAggregatedValueByteSize());
     }
 
     public void testToLongValue() {
-        assertEquals(NumericUtils.doubleToSortableLong(3.0), aggregator.toLongValue(3.0), 0.0);
+        double randomDouble = randomDouble();
+        assertEquals(NumericUtils.doubleToSortableLong(randomDouble), aggregator.toLongValue(randomDouble), 0.0);
     }
 
     public void testToStarTreeNumericTypeValue() {
         MaxValueAggregator aggregator = new MaxValueAggregator(StarTreeNumericType.DOUBLE);
-        assertEquals(NumericUtils.sortableLongToDouble(3L), aggregator.toStarTreeNumericTypeValue(3L), 0.0);
+        long randomLong = randomLong();
+        assertEquals(NumericUtils.sortableLongToDouble(randomLong), aggregator.toStarTreeNumericTypeValue(randomLong), 0.0);
     }
 
     public void testIdentityMetricValue() {
         assertNull(aggregator.getIdentityMetricValue());
     }
+
+    @Override
+    public ValueAggregator getValueAggregator() {
+        return aggregator;
+    }
+
+    @Override
+    public MetricStat getMetricStat() {
+        return MetricStat.MAX;
+    }
+
+    @Override
+    public StarTreeNumericType getValueAggregatorType() {
+        return StarTreeNumericType.DOUBLE;
+    }
 }