diff --git a/docs/reference/release-notes/7.0.0-alpha1.asciidoc b/docs/reference/release-notes/7.0.0-alpha1.asciidoc index 1cc328f16598b..cf2e1e30be050 100644 --- a/docs/reference/release-notes/7.0.0-alpha1.asciidoc +++ b/docs/reference/release-notes/7.0.0-alpha1.asciidoc @@ -16,3 +16,9 @@ Cross-Cluster-Search:: Rest API:: * The Clear Cache API only supports `POST` as HTTP method + +Aggregations:: +* The Percentiles and PercentileRanks aggregations now return `null` in the REST response, + instead of `NaN`. This makes it consistent with the rest of the aggregations. Note: + this only applies to the REST response, the java objects continue to return `NaN` (also + consistent with other aggregations) \ No newline at end of file diff --git a/server/src/main/java/org/elasticsearch/search/DocValueFormat.java b/server/src/main/java/org/elasticsearch/search/DocValueFormat.java index 242e088747341..3a3b1c680aba1 100644 --- a/server/src/main/java/org/elasticsearch/search/DocValueFormat.java +++ b/server/src/main/java/org/elasticsearch/search/DocValueFormat.java @@ -394,6 +394,22 @@ public String format(long value) { @Override public String format(double value) { + /** + * Explicitly check for NaN, since it formats to "�" or "NaN" depending on JDK version. + * + * Decimal formatter uses the JRE's default symbol list (via Locale.ROOT above). In JDK8, + * this translates into using {@link sun.util.locale.provider.JRELocaleProviderAdapter}, which loads + * {@link sun.text.resources.FormatData} for symbols. There, `NaN` is defined as `\ufffd` (�) + * + * In JDK9+, {@link sun.util.cldr.CLDRLocaleProviderAdapter} is used instead, which loads + * {@link sun.text.resources.cldr.FormatData}. There, `NaN` is defined as `"NaN"` + * + * Since the character � isn't very useful, and makes the output change depending on JDK version, + * we manually check to see if the value is NaN and return the string directly. + */ + if (Double.isNaN(value)) { + return String.valueOf(Double.NaN); + } return format.format(value); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/ParsedPercentiles.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/ParsedPercentiles.java index 3f56b21dcd8a0..2c7da76446d5a 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/ParsedPercentiles.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/ParsedPercentiles.java @@ -92,9 +92,9 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) builder.startObject(CommonFields.VALUES.getPreferredName()); for (Map.Entry percentile : percentiles.entrySet()) { Double key = percentile.getKey(); - builder.field(String.valueOf(key), percentile.getValue()); - - if (valuesAsString) { + Double value = percentile.getValue(); + builder.field(String.valueOf(key), value.isNaN() ? null : value); + if (valuesAsString && value.isNaN() == false) { builder.field(key + "_as_string", getPercentileAsString(key)); } } @@ -106,8 +106,9 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) builder.startObject(); { builder.field(CommonFields.KEY.getPreferredName(), key); - builder.field(CommonFields.VALUE.getPreferredName(), percentile.getValue()); - if (valuesAsString) { + Double value = percentile.getValue(); + builder.field(CommonFields.VALUE.getPreferredName(), value.isNaN() ? null : value); + if (valuesAsString && value.isNaN() == false) { builder.field(CommonFields.VALUE_AS_STRING.getPreferredName(), getPercentileAsString(key)); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/AbstractInternalHDRPercentiles.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/AbstractInternalHDRPercentiles.java index 48d35de6cb6ab..a7b359d59373c 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/AbstractInternalHDRPercentiles.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/AbstractInternalHDRPercentiles.java @@ -123,9 +123,9 @@ public XContentBuilder doXContentBody(XContentBuilder builder, Params params) th for(int i = 0; i < keys.length; ++i) { String key = String.valueOf(keys[i]); double value = value(keys[i]); - builder.field(key, value); - if (format != DocValueFormat.RAW) { - builder.field(key + "_as_string", format.format(value)); + builder.field(key, state.getTotalCount() == 0 ? null : value); + if (format != DocValueFormat.RAW && state.getTotalCount() > 0) { + builder.field(key + "_as_string", format.format(value).toString()); } } builder.endObject(); @@ -135,8 +135,8 @@ public XContentBuilder doXContentBody(XContentBuilder builder, Params params) th double value = value(keys[i]); builder.startObject(); builder.field(CommonFields.KEY.getPreferredName(), keys[i]); - builder.field(CommonFields.VALUE.getPreferredName(), value); - if (format != DocValueFormat.RAW) { + builder.field(CommonFields.VALUE.getPreferredName(), state.getTotalCount() == 0 ? null : value); + if (format != DocValueFormat.RAW && state.getTotalCount() > 0) { builder.field(CommonFields.VALUE_AS_STRING.getPreferredName(), format.format(value).toString()); } builder.endObject(); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/tdigest/AbstractInternalTDigestPercentiles.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/tdigest/AbstractInternalTDigestPercentiles.java index 3806d7feb9550..0938710406a7b 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/tdigest/AbstractInternalTDigestPercentiles.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/tdigest/AbstractInternalTDigestPercentiles.java @@ -106,9 +106,9 @@ public XContentBuilder doXContentBody(XContentBuilder builder, Params params) th for(int i = 0; i < keys.length; ++i) { String key = String.valueOf(keys[i]); double value = value(keys[i]); - builder.field(key, value); - if (format != DocValueFormat.RAW) { - builder.field(key + "_as_string", format.format(value)); + builder.field(key, state.size() == 0 ? null : value); + if (format != DocValueFormat.RAW && state.size() > 0) { + builder.field(key + "_as_string", format.format(value).toString()); } } builder.endObject(); @@ -118,8 +118,8 @@ public XContentBuilder doXContentBody(XContentBuilder builder, Params params) th double value = value(keys[i]); builder.startObject(); builder.field(CommonFields.KEY.getPreferredName(), keys[i]); - builder.field(CommonFields.VALUE.getPreferredName(), value); - if (format != DocValueFormat.RAW) { + builder.field(CommonFields.VALUE.getPreferredName(), state.size() == 0 ? null : value); + if (format != DocValueFormat.RAW && state.size() > 0) { builder.field(CommonFields.VALUE_AS_STRING.getPreferredName(), format.format(value).toString()); } builder.endObject(); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/AbstractPercentilesTestCase.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/AbstractPercentilesTestCase.java index e54a2a8b9a14f..c4a3d3b2ffcef 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/AbstractPercentilesTestCase.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/AbstractPercentilesTestCase.java @@ -19,6 +19,10 @@ package org.elasticsearch.search.aggregations.metrics.percentiles; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.aggregations.Aggregation.CommonFields; import org.elasticsearch.search.aggregations.InternalAggregation; @@ -27,11 +31,14 @@ import java.io.IOException; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.function.Predicate; +import static org.hamcrest.Matchers.equalTo; + public abstract class AbstractPercentilesTestCase> extends InternalAggregationTestCase { @@ -49,7 +56,7 @@ public void setUp() throws Exception { @Override protected T createTestInstance(String name, List pipelineAggregators, Map metaData) { - int numValues = randomInt(100); + int numValues = frequently() ? randomInt(100) : 0; double[] values = new double[numValues]; for (int i = 0; i < numValues; ++i) { values[i] = randomDouble(); @@ -89,4 +96,53 @@ public static double[] randomPercents(boolean sorted) { protected Predicate excludePathsFromXContentInsertion() { return path -> path.endsWith(CommonFields.VALUES.getPreferredName()); } + + protected abstract void assertPercentile(T agg, Double value); + + public void testEmptyRanksXContent() throws IOException { + double[] percents = new double[]{1,2,3}; + boolean keyed = randomBoolean(); + DocValueFormat docValueFormat = randomNumericDocValueFormat(); + + T agg = createTestInstance("test", Collections.emptyList(), Collections.emptyMap(), keyed, docValueFormat, percents, new double[0]); + + for (Percentile percentile : agg) { + Double value = percentile.getValue(); + assertPercentile(agg, value); + } + + XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); + builder.startObject(); + agg.doXContentBody(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + String expected; + if (keyed) { + expected = "{\n" + + " \"values\" : {\n" + + " \"1.0\" : null,\n" + + " \"2.0\" : null,\n" + + " \"3.0\" : null\n" + + " }\n" + + "}"; + } else { + expected = "{\n" + + " \"values\" : [\n" + + " {\n" + + " \"key\" : 1.0,\n" + + " \"value\" : null\n" + + " },\n" + + " {\n" + + " \"key\" : 2.0,\n" + + " \"value\" : null\n" + + " },\n" + + " {\n" + + " \"key\" : 3.0,\n" + + " \"value\" : null\n" + + " }\n" + + " ]\n" + + "}"; + } + + assertThat(Strings.toString(builder), equalTo(expected)); + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/InternalPercentilesRanksTestCase.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/InternalPercentilesRanksTestCase.java index f45b7cce51e37..a63fd42da7d96 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/InternalPercentilesRanksTestCase.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/InternalPercentilesRanksTestCase.java @@ -22,6 +22,8 @@ import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.ParsedAggregation; +import static org.hamcrest.Matchers.equalTo; + public abstract class InternalPercentilesRanksTestCase extends AbstractPercentilesTestCase { @@ -39,4 +41,10 @@ protected final void assertFromXContent(T aggregation, ParsedAggregation parsedA Class parsedClass = implementationClass(); assertTrue(parsedClass != null && parsedClass.isInstance(parsedAggregation)); } + + @Override + protected void assertPercentile(T agg, Double value) { + assertThat(agg.percent(value), equalTo(Double.NaN)); + assertThat(agg.percentAsString(value), equalTo("NaN")); + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/InternalPercentilesTestCase.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/InternalPercentilesTestCase.java index be105f2af80b6..1024577a6b6ed 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/InternalPercentilesTestCase.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/InternalPercentilesTestCase.java @@ -24,6 +24,8 @@ import java.util.List; +import static org.hamcrest.Matchers.equalTo; + public abstract class InternalPercentilesTestCase extends AbstractPercentilesTestCase { @Override @@ -49,4 +51,10 @@ public static double[] randomPercents() { } return percents; } + + @Override + protected void assertPercentile(T agg, Double value) { + assertThat(agg.percentile(value), equalTo(Double.NaN)); + assertThat(agg.percentileAsString(value), equalTo("NaN")); + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/InternalHDRPercentilesRanksTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/InternalHDRPercentilesRanksTests.java index dcbd5cdbd5a3a..ee0e3602f2039 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/InternalHDRPercentilesRanksTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/InternalHDRPercentilesRanksTests.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; + public class InternalHDRPercentilesRanksTests extends InternalPercentilesRanksTestCase { @Override