Don't validate internal stats if they are empty (#113846)

Fixes a validation step that might prevent creating empty aggregations iif the output format does not allows negative numbers.
elastic · Oct 4, 2024 · 0ef5219 · 0ef5219
1 parent e32a70a
commit 0ef5219
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 1 deletion.
diff --git a/docs/changelog/113846.yaml b/docs/changelog/113846.yaml
@@ -0,0 +1,6 @@
+pr: 113846
+summary: Don't validate internal stats if they are empty
+area: Aggregations
+type: bug
+issues:
+ - 113811
diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalStats.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/InternalStats.java
@@ -76,7 +76,7 @@ public InternalStats(
     }
 
     private void verifyFormattingStats() {
-        if (format != DocValueFormat.RAW) {
+        if (format != DocValueFormat.RAW && count != 0) {
             verifyFormattingStat(Fields.MIN, format, min);
             verifyFormattingStat(Fields.MAX, format, max);
             verifyFormattingStat(Fields.AVG, format, getAvg());

diff --git a/...test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsAggregatorTests.java b/...test/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsAggregatorTests.java
@@ -14,16 +14,20 @@
 import org.apache.lucene.document.SortedNumericDocValuesField;
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.util.NumericUtils;
+import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.core.CheckedConsumer;
+import org.elasticsearch.index.mapper.DateFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.NumberFieldMapper;
 import org.elasticsearch.search.aggregations.AggregatorTestCase;
 import org.elasticsearch.search.aggregations.support.AggregationInspectionHelper;
 
 import java.io.IOException;
+import java.util.Map;
 import java.util.function.Consumer;
 
 import static java.util.Collections.singleton;
+import static org.elasticsearch.search.aggregations.AggregationBuilders.stats;
 
 public class ExtendedStatsAggregatorTests extends AggregatorTestCase {
     private static final double TOLERANCE = 1e-5;
@@ -49,6 +53,37 @@ public void testEmpty() throws IOException {
         });
     }
 
+    public void testEmptyDate() throws IOException {
+        DateFormatter.forPattern("epoch_millis");
+        final MappedFieldType ft = new DateFieldMapper.DateFieldType(
+            "field",
+            true,
+            true,
+            false,
+            true,
+            DateFormatter.forPattern("epoch_millis"),
+            DateFieldMapper.Resolution.MILLISECONDS,
+            null,
+            null,
+            Map.of()
+        );
+        testCase(ft, iw -> {}, stats -> {
+            assertEquals(0d, stats.getCount(), 0);
+            assertEquals(0d, stats.getSum(), 0);
+            assertEquals(Float.NaN, stats.getAvg(), 0);
+            assertEquals(Double.POSITIVE_INFINITY, stats.getMin(), 0);
+            assertEquals(Double.NEGATIVE_INFINITY, stats.getMax(), 0);
+            assertEquals(Double.NaN, stats.getVariance(), 0);
+            assertEquals(Double.NaN, stats.getVariancePopulation(), 0);
+            assertEquals(Double.NaN, stats.getVarianceSampling(), 0);
+            assertEquals(Double.NaN, stats.getStdDeviation(), 0);
+            assertEquals(Double.NaN, stats.getStdDeviationPopulation(), 0);
+            assertEquals(Double.NaN, stats.getStdDeviationSampling(), 0);
+            assertEquals(0d, stats.getSumOfSquares(), 0);
+            assertFalse(AggregationInspectionHelper.hasValue(stats));
+        });
+    }
+
     public void testRandomDoubles() throws IOException {
         MappedFieldType ft = new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.DOUBLE);
         final ExtendedSimpleStatsAggregator expected = new ExtendedSimpleStatsAggregator();

diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorTests.java
@@ -18,7 +18,9 @@
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.util.NumericUtils;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.core.CheckedConsumer;
+import org.elasticsearch.index.mapper.DateFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.NumberFieldMapper;
 import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
@@ -73,6 +75,30 @@ public void testEmpty() throws IOException {
         }, ft);
     }
 
+    public void testEmptyDate() throws IOException {
+        DateFormatter.forPattern("epoch_millis");
+        final MappedFieldType ft = new DateFieldMapper.DateFieldType(
+            "field",
+            true,
+            true,
+            false,
+            true,
+            DateFormatter.forPattern("epoch_millis"),
+            DateFieldMapper.Resolution.MILLISECONDS,
+            null,
+            null,
+            Map.of()
+        );
+        testCase(stats("_name").field(ft.name()), iw -> {}, stats -> {
+            assertEquals(0d, stats.getCount(), 0);
+            assertEquals(0d, stats.getSum(), 0);
+            assertEquals(Float.NaN, stats.getAvg(), 0);
+            assertEquals(Double.POSITIVE_INFINITY, stats.getMin(), 0);
+            assertEquals(Double.NEGATIVE_INFINITY, stats.getMax(), 0);
+            assertFalse(AggregationInspectionHelper.hasValue(stats));
+        }, ft);
+    }
+
     public void testRandomDoubles() throws IOException {
         final MappedFieldType ft = new NumberFieldMapper.NumberFieldType("field", NumberType.DOUBLE);
         final SimpleStatsAggregator expected = new SimpleStatsAggregator();