From fcea4dec13707f7aff4a877b310f4ac30a3397c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 20 Apr 2017 14:48:22 +0200 Subject: [PATCH 1/4] Add parsing for InternalStats --- .../aggregations/ParsedAggregation.java | 11 ++ ...dSingleValueNumericMetricsAggregation.java | 51 +++--- .../metrics/stats/ParsedStats.java | 145 ++++++++++++++++++ .../InternalAggregationTestCase.java | 3 + .../metrics/InternalStatsTests.java | 33 +++- 5 files changed, 209 insertions(+), 34 deletions(-) create mode 100644 core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/ParsedAggregation.java b/core/src/main/java/org/elasticsearch/search/aggregations/ParsedAggregation.java index 452fe9dcb0853..d451cfb5eaffd 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/ParsedAggregation.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/ParsedAggregation.java @@ -22,6 +22,8 @@ import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentParser.Token; import java.io.IOException; import java.util.Collections; @@ -77,4 +79,13 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par } protected abstract XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException; + + protected static double parseValue(XContentParser parser, double defaultNullValue) throws IOException { + Token currentToken = parser.currentToken(); + if (currentToken == XContentParser.Token.VALUE_NUMBER || currentToken == XContentParser.Token.VALUE_STRING) { + return parser.doubleValue(); + } else { + return defaultNullValue; + } + } } \ No newline at end of file diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedSingleValueNumericMetricsAggregation.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedSingleValueNumericMetricsAggregation.java index 5eb0a7223a11f..2e6e468b8928c 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedSingleValueNumericMetricsAggregation.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedSingleValueNumericMetricsAggregation.java @@ -20,48 +20,35 @@ import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.ObjectParser.ValueType; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentParser.Token; import org.elasticsearch.search.aggregations.ParsedAggregation; -import java.io.IOException; - public abstract class ParsedSingleValueNumericMetricsAggregation extends ParsedAggregation implements NumericMetricsAggregation.SingleValue { - protected double value; - protected String valueAsString; - - @Override - public String getValueAsString() { - if (valueAsString != null) { - return valueAsString; - } else { - return Double.toString(value); - } - } + protected double value; + protected String valueAsString; - @Override - public double value() { - return value; + @Override + public String getValueAsString() { + if (valueAsString != null) { + return valueAsString; + } else { + return Double.toString(value); } + } - protected void setValue(double value) { - this.value = value; - } + @Override + public double value() { + return value; + } - protected void setValueAsString(String valueAsString) { - this.valueAsString = valueAsString; - } + protected void setValue(double value) { + this.value = value; + } - protected static double parseValue(XContentParser parser, double defaultNullValue) throws IOException { - Token currentToken = parser.currentToken(); - if (currentToken == XContentParser.Token.VALUE_NUMBER || currentToken == XContentParser.Token.VALUE_STRING) { - return parser.doubleValue(); - } else { - return defaultNullValue; - } - } + protected void setValueAsString(String valueAsString) { + this.valueAsString = valueAsString; + } protected static void declareSingleValueFields(ObjectParser objectParser, double defaultNullValue) { diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java new file mode 100644 index 0000000000000..1f9335fb8202f --- /dev/null +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java @@ -0,0 +1,145 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.aggregations.metrics.stats; + +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.ObjectParser.ValueType; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.search.aggregations.ParsedAggregation; +import org.elasticsearch.search.aggregations.metrics.stats.InternalStats.Fields; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +public class ParsedStats extends ParsedAggregation implements Stats { + + protected long count; + protected double min; + protected double max; + protected double sum; + protected double avg; + + protected Map valueAsString = new HashMap<>(); + + @Override + public long getCount() { + return count; + } + + @Override + public double getMin() { + return min; + } + + @Override + public double getMax() { + return max; + } + + @Override + public double getAvg() { + return avg; + } + + @Override + public double getSum() { + return sum; + } + + @Override + public String getCountAsString() { + return Double.toString(count); + } + + @Override + public String getMinAsString() { + return valueAsString.getOrDefault(Fields.MIN_AS_STRING, Double.toString(min)); + } + + @Override + public String getMaxAsString() { + return valueAsString.getOrDefault(Fields.MAX_AS_STRING, Double.toString(max)); + } + + @Override + public String getAvgAsString() { + return valueAsString.getOrDefault(Fields.AVG_AS_STRING, Double.toString(avg)); + } + + @Override + public String getSumAsString() { + return valueAsString.getOrDefault(Fields.SUM_AS_STRING, Double.toString(sum)); + } + + @Override + protected String getType() { + return StatsAggregationBuilder.NAME; + } + + @Override + protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { + builder.field(Fields.COUNT, count); + builder.field(Fields.MIN, count != 0 ? min : null); + builder.field(Fields.MAX, count != 0 ? max : null); + builder.field(Fields.AVG, count != 0 ? avg : null); + builder.field(Fields.SUM, count != 0 ? sum : null); + if (count != 0 && valueAsString.get(Fields.MIN_AS_STRING) != null) { + builder.field(Fields.MIN_AS_STRING, getMinAsString()); + builder.field(Fields.MAX_AS_STRING, getMaxAsString()); + builder.field(Fields.AVG_AS_STRING, getAvgAsString()); + builder.field(Fields.SUM_AS_STRING, getSumAsString()); + } + otherStatsToXCotent(builder, params); + return builder; + } + + private static final ObjectParser PARSER = new ObjectParser<>(ParsedStats.class.getSimpleName(), true, + ParsedStats::new); + + static { + declareAggregationFields(PARSER); + PARSER.declareLong((agg, value) -> agg.count = value, new ParseField(Fields.COUNT)); + PARSER.declareField((agg, value) -> agg.min = value, (parser, context) -> parseValue(parser, Double.POSITIVE_INFINITY), + new ParseField(Fields.MIN), ValueType.DOUBLE_OR_NULL); + PARSER.declareField((agg, value) -> agg.max = value, (parser, context) -> parseValue(parser, Double.NEGATIVE_INFINITY), + new ParseField(Fields.MAX), ValueType.DOUBLE_OR_NULL); + PARSER.declareField((agg, value) -> agg.avg = value, (parser, context) -> parseValue(parser, 0), new ParseField(Fields.AVG), + ValueType.DOUBLE_OR_NULL); + PARSER.declareField((agg, value) -> agg.sum = value, (parser, context) -> parseValue(parser, 0), new ParseField(Fields.SUM), + ValueType.DOUBLE_OR_NULL); + PARSER.declareString((agg, value) -> agg.valueAsString.put(Fields.MIN_AS_STRING, value), new ParseField(Fields.MIN_AS_STRING)); + PARSER.declareString((agg, value) -> agg.valueAsString.put(Fields.MAX_AS_STRING, value), new ParseField(Fields.MAX_AS_STRING)); + PARSER.declareString((agg, value) -> agg.valueAsString.put(Fields.AVG_AS_STRING, value), new ParseField(Fields.AVG_AS_STRING)); + PARSER.declareString((agg, value) -> agg.valueAsString.put(Fields.SUM_AS_STRING, value), new ParseField(Fields.SUM_AS_STRING)); + } + + public static ParsedStats fromXContent(XContentParser parser, final String name) { + ParsedStats parsedStats = PARSER.apply(parser, null); + parsedStats.setName(name); + return parsedStats; + } + + protected XContentBuilder otherStatsToXCotent(XContentBuilder builder, Params params) throws IOException { + return builder; + } +} diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationTestCase.java b/core/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationTestCase.java index fd6b9ecdc44c0..63211f3db222a 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationTestCase.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationTestCase.java @@ -50,6 +50,8 @@ import org.elasticsearch.search.aggregations.metrics.percentiles.tdigest.InternalTDigestPercentiles; import org.elasticsearch.search.aggregations.metrics.percentiles.tdigest.ParsedTDigestPercentileRanks; import org.elasticsearch.search.aggregations.metrics.percentiles.tdigest.ParsedTDigestPercentiles; +import org.elasticsearch.search.aggregations.metrics.stats.ParsedStats; +import org.elasticsearch.search.aggregations.metrics.stats.StatsAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.sum.ParsedSum; import org.elasticsearch.search.aggregations.metrics.sum.SumAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.valuecount.ParsedValueCount; @@ -99,6 +101,7 @@ static List getNamedXContents() { namedXContents.put(InternalSimpleValue.NAME, (p, c) -> ParsedSimpleValue.fromXContent(p, (String) c)); namedXContents.put(DerivativePipelineAggregationBuilder.NAME, (p, c) -> ParsedDerivative.fromXContent(p, (String) c)); namedXContents.put(InternalBucketMetricValue.NAME, (p, c) -> ParsedBucketMetricValue.fromXContent(p, (String) c)); + namedXContents.put(StatsAggregationBuilder.NAME, (p, c) -> ParsedStats.fromXContent(p, (String) c)); return namedXContents.entrySet().stream() .map(entry -> new NamedXContentRegistry.Entry(Aggregation.class, new ParseField(entry.getKey()), entry.getValue())) diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalStatsTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalStatsTests.java index c99d208581c13..e294f3d1c6038 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalStatsTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalStatsTests.java @@ -21,7 +21,9 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.aggregations.InternalAggregationTestCase; +import org.elasticsearch.search.aggregations.ParsedAggregation; import org.elasticsearch.search.aggregations.metrics.stats.InternalStats; +import org.elasticsearch.search.aggregations.metrics.stats.ParsedStats; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import java.util.Collections; @@ -29,9 +31,9 @@ import java.util.Map; public class InternalStatsTests extends InternalAggregationTestCase { + @Override - protected InternalStats createTestInstance(String name, List pipelineAggregators, - Map metaData) { + protected InternalStats createTestInstance(String name, List pipelineAggregators, Map metaData) { long count = frequently() ? randomIntBetween(1, Integer.MAX_VALUE) : 0; double min = randomDoubleBetween(-1000000, 1000000, true); double max = randomDoubleBetween(-1000000, 1000000, true); @@ -62,8 +64,35 @@ protected void assertReduced(InternalStats reduced, List inputs) assertEquals(expectedMax, reduced.getMax(), 0d); } + @Override + protected void assertFromXContent(InternalStats aggregation, ParsedAggregation parsedAggregation) { + assertTrue(parsedAggregation instanceof ParsedStats); + ParsedStats parsed = (ParsedStats) parsedAggregation; + long count = aggregation.getCount(); + assertEquals(count, parsed.getCount()); + // for count == 0, fields are rendered as `null`, so we test that we parse to default values used also in the reduce phase + assertEquals(count > 0 ? aggregation.getMin() : Double.POSITIVE_INFINITY , parsed.getMin(), 0); + assertEquals(count > 0 ? aggregation.getMax() : Double.NEGATIVE_INFINITY, parsed.getMax(), 0); + assertEquals(count > 0 ? aggregation.getSum() : 0, parsed.getSum(), 0); + assertEquals(count > 0 ? aggregation.getAvg() : 0, parsed.getAvg(), 0); + // also as_string values are only rendered for count != 0 + if (count > 0) { + assertEquals(aggregation.getMinAsString(), parsed.getMinAsString()); + assertEquals(aggregation.getMaxAsString(), parsed.getMaxAsString()); + assertEquals(aggregation.getSumAsString(), parsed.getSumAsString()); + assertEquals(aggregation.getAvgAsString(), parsed.getAvgAsString()); + // NORELEASE there is no COUNT_AS_STRING in the json output, so we cannot get back a formatted value here + if (aggregation.format.equals(DocValueFormat.RAW)) { + assertEquals(aggregation.getCountAsString(), parsed.getCountAsString()); + } else { + assertEquals(Double.toString(aggregation.getCount()), parsed.getCountAsString()); + } + } + } + @Override protected Writeable.Reader instanceReader() { return InternalStats::new; } } + From 43ae78cee504e586a84358cb902de5f86b9daca7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 21 Apr 2017 17:11:01 +0200 Subject: [PATCH 2/4] Adressing tlrx review comments --- .../aggregations/ParsedAggregation.java | 6 ++- ...dSingleValueNumericMetricsAggregation.java | 2 +- .../metrics/stats/InternalStats.java | 4 +- .../metrics/stats/ParsedStats.java | 39 +++++++++++-------- .../stats/extended/InternalExtendedStats.java | 2 +- .../pipeline/derivative/ParsedDerivative.java | 2 +- 6 files changed, 33 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/ParsedAggregation.java b/core/src/main/java/org/elasticsearch/search/aggregations/ParsedAggregation.java index d451cfb5eaffd..6942b6aec5d1f 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/ParsedAggregation.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/ParsedAggregation.java @@ -80,7 +80,11 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par protected abstract XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException; - protected static double parseValue(XContentParser parser, double defaultNullValue) throws IOException { + /** + * Parse a token of type XContentParser.Token.VALUE_NUMBER or XContentParser.Token.STRING to a double. + * In other cases the default value is returned instead. + */ + protected static double parseDouble(XContentParser parser, double defaultNullValue) throws IOException { Token currentToken = parser.currentToken(); if (currentToken == XContentParser.Token.VALUE_NUMBER || currentToken == XContentParser.Token.VALUE_STRING) { return parser.doubleValue(); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedSingleValueNumericMetricsAggregation.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedSingleValueNumericMetricsAggregation.java index 2e6e468b8928c..3105a785e1903 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedSingleValueNumericMetricsAggregation.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedSingleValueNumericMetricsAggregation.java @@ -54,7 +54,7 @@ protected static void declareSingleValueFields(ObjectParser parseValue(parser, defaultNullValue), CommonFields.VALUE, ValueType.DOUBLE_OR_NULL); + (parser, context) -> parseDouble(parser, defaultNullValue), CommonFields.VALUE, ValueType.DOUBLE_OR_NULL); objectParser.declareString(ParsedSingleValueNumericMetricsAggregation::setValueAsString, CommonFields.VALUE_AS_STRING); } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java index b0b2ea73d3c9f..f38746d256701 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java @@ -187,11 +187,11 @@ public XContentBuilder doXContentBody(XContentBuilder builder, Params params) th builder.field(Fields.AVG_AS_STRING, format.format(getAvg())); builder.field(Fields.SUM_AS_STRING, format.format(sum)); } - otherStatsToXCotent(builder, params); + otherStatsToXContent(builder, params); return builder; } - protected XContentBuilder otherStatsToXCotent(XContentBuilder builder, Params params) throws IOException { + protected XContentBuilder otherStatsToXContent(XContentBuilder builder, Params params) throws IOException { return builder; } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java index 1f9335fb8202f..c1c8bdc440009 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java @@ -39,7 +39,7 @@ public class ParsedStats extends ParsedAggregation implements Stats { protected double sum; protected double avg; - protected Map valueAsString = new HashMap<>(); + protected final Map valueAsString = new HashMap<>(); @Override public long getCount() { @@ -99,17 +99,24 @@ protected String getType() { @Override protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { builder.field(Fields.COUNT, count); - builder.field(Fields.MIN, count != 0 ? min : null); - builder.field(Fields.MAX, count != 0 ? max : null); - builder.field(Fields.AVG, count != 0 ? avg : null); - builder.field(Fields.SUM, count != 0 ? sum : null); - if (count != 0 && valueAsString.get(Fields.MIN_AS_STRING) != null) { - builder.field(Fields.MIN_AS_STRING, getMinAsString()); - builder.field(Fields.MAX_AS_STRING, getMaxAsString()); - builder.field(Fields.AVG_AS_STRING, getAvgAsString()); - builder.field(Fields.SUM_AS_STRING, getSumAsString()); + if (count != 0) { + builder.field(Fields.MIN, min); + builder.field(Fields.MAX, max); + builder.field(Fields.AVG, avg); + builder.field(Fields.SUM, sum); + if (valueAsString.get(Fields.MIN_AS_STRING) != null) { + builder.field(Fields.MIN_AS_STRING, getMinAsString()); + builder.field(Fields.MAX_AS_STRING, getMaxAsString()); + builder.field(Fields.AVG_AS_STRING, getAvgAsString()); + builder.field(Fields.SUM_AS_STRING, getSumAsString()); + } + } else { + builder.nullField(Fields.MIN); + builder.nullField(Fields.MAX); + builder.nullField(Fields.AVG); + builder.nullField(Fields.SUM); } - otherStatsToXCotent(builder, params); + otherStatsToXContent(builder, params); return builder; } @@ -119,13 +126,13 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) static { declareAggregationFields(PARSER); PARSER.declareLong((agg, value) -> agg.count = value, new ParseField(Fields.COUNT)); - PARSER.declareField((agg, value) -> agg.min = value, (parser, context) -> parseValue(parser, Double.POSITIVE_INFINITY), + PARSER.declareField((agg, value) -> agg.min = value, (parser, context) -> parseDouble(parser, Double.POSITIVE_INFINITY), new ParseField(Fields.MIN), ValueType.DOUBLE_OR_NULL); - PARSER.declareField((agg, value) -> agg.max = value, (parser, context) -> parseValue(parser, Double.NEGATIVE_INFINITY), + PARSER.declareField((agg, value) -> agg.max = value, (parser, context) -> parseDouble(parser, Double.NEGATIVE_INFINITY), new ParseField(Fields.MAX), ValueType.DOUBLE_OR_NULL); - PARSER.declareField((agg, value) -> agg.avg = value, (parser, context) -> parseValue(parser, 0), new ParseField(Fields.AVG), + PARSER.declareField((agg, value) -> agg.avg = value, (parser, context) -> parseDouble(parser, 0), new ParseField(Fields.AVG), ValueType.DOUBLE_OR_NULL); - PARSER.declareField((agg, value) -> agg.sum = value, (parser, context) -> parseValue(parser, 0), new ParseField(Fields.SUM), + PARSER.declareField((agg, value) -> agg.sum = value, (parser, context) -> parseDouble(parser, 0), new ParseField(Fields.SUM), ValueType.DOUBLE_OR_NULL); PARSER.declareString((agg, value) -> agg.valueAsString.put(Fields.MIN_AS_STRING, value), new ParseField(Fields.MIN_AS_STRING)); PARSER.declareString((agg, value) -> agg.valueAsString.put(Fields.MAX_AS_STRING, value), new ParseField(Fields.MAX_AS_STRING)); @@ -139,7 +146,7 @@ public static ParsedStats fromXContent(XContentParser parser, final String name) return parsedStats; } - protected XContentBuilder otherStatsToXCotent(XContentBuilder builder, Params params) throws IOException { + protected XContentBuilder otherStatsToXContent(XContentBuilder builder, Params params) throws IOException { return builder; } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/extended/InternalExtendedStats.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/extended/InternalExtendedStats.java index 370399bfbb8db..b769850b817d8 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/extended/InternalExtendedStats.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/extended/InternalExtendedStats.java @@ -169,7 +169,7 @@ static class Fields { } @Override - protected XContentBuilder otherStatsToXCotent(XContentBuilder builder, Params params) throws IOException { + protected XContentBuilder otherStatsToXContent(XContentBuilder builder, Params params) throws IOException { builder.field(Fields.SUM_OF_SQRS, count != 0 ? sumOfSqrs : null); builder.field(Fields.VARIANCE, count != 0 ? getVariance() : null); builder.field(Fields.STD_DEVIATION, count != 0 ? getStdDeviation() : null); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/derivative/ParsedDerivative.java b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/derivative/ParsedDerivative.java index ed463239b2440..5469b8b65cfe8 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/derivative/ParsedDerivative.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/pipeline/derivative/ParsedDerivative.java @@ -54,7 +54,7 @@ protected String getType() { PARSER.declareField((agg, normalized) -> { agg.normalizedValue = normalized; agg.hasNormalizationFactor = true; - }, (parser, context) -> parseValue(parser, Double.NaN), NORMALIZED, ValueType.DOUBLE_OR_NULL); + }, (parser, context) -> parseDouble(parser, Double.NaN), NORMALIZED, ValueType.DOUBLE_OR_NULL); PARSER.declareString((agg, normalAsString) -> agg.normalizedAsString = normalAsString, NORMALIZED_AS_STRING); } From 76e5df710648992a149227aac1433e76c8a6f204 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 24 Apr 2017 18:50:51 +0200 Subject: [PATCH 3/4] Removing getCountAsString() and tests for it --- .../search/aggregations/metrics/stats/ParsedStats.java | 5 ----- .../search/aggregations/metrics/InternalStatsTests.java | 6 ------ 2 files changed, 11 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java index c1c8bdc440009..28ca3418a994a 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/ParsedStats.java @@ -66,11 +66,6 @@ public double getSum() { return sum; } - @Override - public String getCountAsString() { - return Double.toString(count); - } - @Override public String getMinAsString() { return valueAsString.getOrDefault(Fields.MIN_AS_STRING, Double.toString(min)); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalStatsTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalStatsTests.java index e294f3d1c6038..c0c54d3a8a000 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalStatsTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalStatsTests.java @@ -81,12 +81,6 @@ protected void assertFromXContent(InternalStats aggregation, ParsedAggregation p assertEquals(aggregation.getMaxAsString(), parsed.getMaxAsString()); assertEquals(aggregation.getSumAsString(), parsed.getSumAsString()); assertEquals(aggregation.getAvgAsString(), parsed.getAvgAsString()); - // NORELEASE there is no COUNT_AS_STRING in the json output, so we cannot get back a formatted value here - if (aggregation.format.equals(DocValueFormat.RAW)) { - assertEquals(aggregation.getCountAsString(), parsed.getCountAsString()); - } else { - assertEquals(Double.toString(aggregation.getCount()), parsed.getCountAsString()); - } } } From b230e0e924baaaba8c909b58d67ecd6b3c26c7ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 25 Apr 2017 12:51:49 +0200 Subject: [PATCH 4/4] Also change InternalStats#doXContentBody() --- .../metrics/stats/InternalStats.java | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java index f38746d256701..a29754ea559a6 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java @@ -177,15 +177,22 @@ static class Fields { @Override public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException { builder.field(Fields.COUNT, count); - builder.field(Fields.MIN, count != 0 ? min : null); - builder.field(Fields.MAX, count != 0 ? max : null); - builder.field(Fields.AVG, count != 0 ? getAvg() : null); - builder.field(Fields.SUM, count != 0 ? sum : null); - if (count != 0 && format != DocValueFormat.RAW) { - builder.field(Fields.MIN_AS_STRING, format.format(min)); - builder.field(Fields.MAX_AS_STRING, format.format(max)); - builder.field(Fields.AVG_AS_STRING, format.format(getAvg())); - builder.field(Fields.SUM_AS_STRING, format.format(sum)); + if (count != 0) { + builder.field(Fields.MIN, min); + builder.field(Fields.MAX, max); + builder.field(Fields.AVG, getAvg()); + builder.field(Fields.SUM, sum); + if (format != DocValueFormat.RAW) { + builder.field(Fields.MIN_AS_STRING, format.format(min)); + builder.field(Fields.MAX_AS_STRING, format.format(max)); + builder.field(Fields.AVG_AS_STRING, format.format(getAvg())); + builder.field(Fields.SUM_AS_STRING, format.format(sum)); + } + } else { + builder.nullField(Fields.MIN); + builder.nullField(Fields.MAX); + builder.nullField(Fields.AVG); + builder.nullField(Fields.SUM); } otherStatsToXContent(builder, params); return builder;