diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java index 8860e1b52e6f7..29f36f117fff1 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java @@ -181,6 +181,12 @@ public PercentilesAggregationBuilder percentiles(double... percents) { } double[] sortedPercents = Arrays.copyOf(percents, percents.length); Arrays.sort(sortedPercents); + for (double percent : sortedPercents) { + if (percent < 0.0 || percent > 100.0) { + throw new IllegalArgumentException("percent must be in [0,100], got [" + percent + "]: [" + name + "]"); + } + } + this.percents = sortedPercents; return this; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java index 3bab055161aee..94f5ae0fb606a 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesIT.java @@ -26,19 +26,21 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.aggregations.AggregationTestScriptsPlugin; +import org.elasticsearch.search.aggregations.BucketOrder; import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.bucket.filter.Filter; import org.elasticsearch.search.aggregations.bucket.global.Global; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.terms.Terms; -import org.elasticsearch.search.aggregations.BucketOrder; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static java.util.Collections.emptyMap; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; @@ -67,21 +69,21 @@ protected Collection> nodePlugins() { private static double[] randomPercentiles() { final int length = randomIntBetween(1, 20); - final double[] percentiles = new double[length]; - for (int i = 0; i < percentiles.length; ++i) { + final Set uniquedPercentiles = new HashSet<>(); + while (uniquedPercentiles.size() < length) { switch (randomInt(20)) { case 0: - percentiles[i] = 0; + uniquedPercentiles.add(0.0); break; case 1: - percentiles[i] = 100; + uniquedPercentiles.add(100.0); break; default: - percentiles[i] = randomDouble() * 100; + uniquedPercentiles.add(randomDouble() * 100); break; } } - Arrays.sort(percentiles); + double[] percentiles= uniquedPercentiles.stream().mapToDouble(Double::doubleValue).sorted().toArray(); LogManager.getLogger(HDRPercentilesIT.class).info("Using percentiles={}", Arrays.toString(percentiles)); return percentiles; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/PercentilesTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/PercentilesTests.java index 2f8d22889e317..14036421ae935 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/PercentilesTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/PercentilesTests.java @@ -70,6 +70,15 @@ public void testNullOrEmptyPercentilesThrows() throws IOException { assertEquals("[percents] must not be empty: [testAgg]", ex.getMessage()); } + public void testOutOfRangePercentilesThrows() throws IOException { + PercentilesAggregationBuilder builder = new PercentilesAggregationBuilder("testAgg"); + IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> builder.percentiles(-0.4)); + assertEquals("percent must be in [0,100], got [-0.4]: [testAgg]", ex.getMessage()); + + ex = expectThrows(IllegalArgumentException.class, () -> builder.percentiles(104)); + assertEquals("percent must be in [0,100], got [104.0]: [testAgg]", ex.getMessage()); + } + public void testExceptionMultipleMethods() throws IOException { final String illegalAgg = "{\n" + " \"percentiles\": {\n" + diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java index 88a2b450eac17..7c607de570ccc 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesIT.java @@ -26,19 +26,21 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.aggregations.AggregationTestScriptsPlugin; +import org.elasticsearch.search.aggregations.BucketOrder; import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.bucket.filter.Filter; import org.elasticsearch.search.aggregations.bucket.global.Global; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.terms.Terms; -import org.elasticsearch.search.aggregations.BucketOrder; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static java.util.Collections.emptyMap; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; @@ -66,21 +68,21 @@ protected Collection> nodePlugins() { private static double[] randomPercentiles() { final int length = randomIntBetween(1, 20); - final double[] percentiles = new double[length]; - for (int i = 0; i < percentiles.length; ++i) { + final Set uniquedPercentiles = new HashSet<>(); + for (int i = 0; i < length; ++i) { switch (randomInt(20)) { case 0: - percentiles[i] = 0; + uniquedPercentiles.add(0.0); break; case 1: - percentiles[i] = 100; + uniquedPercentiles.add(100.0); break; default: - percentiles[i] = randomDouble() * 100; + uniquedPercentiles.add(randomDouble() * 100); break; } } - Arrays.sort(percentiles); + double[] percentiles= uniquedPercentiles.stream().mapToDouble(Double::doubleValue).sorted().toArray(); LogManager.getLogger(TDigestPercentilesIT.class).info("Using percentiles={}", Arrays.toString(percentiles)); return percentiles; } diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramPercentileAggregationTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramPercentileAggregationTests.java index e33f5dae95c1e..6d95f74e4428a 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramPercentileAggregationTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramPercentileAggregationTests.java @@ -7,11 +7,11 @@ import com.tdunning.math.stats.Centroid; + import org.HdrHistogram.DoubleHistogram; import org.HdrHistogram.DoubleHistogramIterationValue; import org.apache.lucene.util.TestUtil; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; - import org.elasticsearch.action.admin.indices.refresh.RefreshRequest; import org.elasticsearch.action.bulk.BulkRequest; import org.elasticsearch.action.index.IndexRequest; @@ -19,7 +19,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.plugins.Plugin; - import org.elasticsearch.search.aggregations.AggregationBuilders; import org.elasticsearch.search.aggregations.metrics.InternalHDRPercentiles; import org.elasticsearch.search.aggregations.metrics.InternalTDigestPercentiles; @@ -222,7 +221,7 @@ public void testTDigestHistogram() throws Exception { PercentilesAggregationBuilder builder = AggregationBuilders.percentiles("agg").field("inner.data").method(PercentilesMethod.TDIGEST) - .compression(compression).percentiles(10, 25, 500, 75); + .compression(compression).percentiles(10, 25, 50, 75); SearchResponse responseRaw = client().prepareSearch("raw").addAggregation(builder).get(); SearchResponse responsePreAgg = client().prepareSearch("pre_agg").addAggregation(builder).get();