Skip to content

Commit

Permalink
Percentiles aggregation: disallow specifying same percentile v… (#52257)
Browse files Browse the repository at this point in the history
Disallow specifying the same percentile multiple times in percentiles
aggregation

Related: #51871
  • Loading branch information
Hendrik Muhs committed Feb 26, 2020
1 parent 451c341 commit 854f698
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 1 deletion.
2 changes: 2 additions & 0 deletions docs/reference/migration/migrate_8_0.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ See also <<release-highlights>> and <<es-release-notes>>.

coming[8.0.0]

* <<breaking_80_aggregations_changes>>
* <<breaking_80_analysis_changes>>
* <<breaking_80_allocation_changes>>
* <<breaking_80_breaker_changes>>
Expand Down Expand Up @@ -63,6 +64,7 @@ is replaced with a new endpoint that does not contain `_xpack`. As an example,

// end::notable-breaking-changes[]

include::migrate_8_0/aggregations.asciidoc[]
include::migrate_8_0/analysis.asciidoc[]
include::migrate_8_0/allocation.asciidoc[]
include::migrate_8_0/breaker.asciidoc[]
Expand Down
17 changes: 17 additions & 0 deletions docs/reference/migration/migrate_8_0/aggregations.asciidoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[float]
[[breaking_80_aggregations_changes]]
=== Aggregations changes

//NOTE: The notable-breaking-changes tagged regions are re-used in the
//Installation and Upgrade Guide

//tag::notable-breaking-changes[]
[discrete]
[[percentile-duplication]]
==== Duplicate values no longer supported in percentiles aggregation

If you specify the `percents` parameter with the
<<search-aggregations-metrics-percentile-aggregation,percentiles aggregation>>,
its values must be unique. Otherwise, an exception occurs.

// end::notable-breaking-changes[]
8 changes: 7 additions & 1 deletion docs/reference/release-notes/8.0.0-alpha1.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,11 @@
The changes listed below have been released for the first time in {es}
8.0.0-alpha1.

coming[8.0.0]
[[breaking-8.0.0-alpha1]]
[float]
=== Breaking changes

Aggregations::
* Disallow specifying the same percentile multiple times in percentiles aggregation {pull}52257[#52257]

coming[8.0.0]
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,17 @@ private static double[] validatePercentiles(double[] percents, String aggName) {
throw new IllegalArgumentException("[percents] must not be empty: [" + aggName + "]");
}
double[] sortedPercents = Arrays.copyOf(percents, percents.length);
double previousPercent = -1.0;
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 + "]: [" + aggName + "]");
}

if (percent == previousPercent) {
throw new IllegalArgumentException("percent [" + percent + "] has been specified twice: [" + aggName + "]");
}
previousPercent = percent;
}
return sortedPercents;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ public void testOutOfRangePercentilesThrows() throws IOException {
assertEquals("percent must be in [0,100], got [104.0]: [testAgg]", ex.getMessage());
}

public void testDuplicatePercentilesThrows() throws IOException {
PercentilesAggregationBuilder builder = new PercentilesAggregationBuilder("testAgg");
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> builder.percentiles(5, 42, 10, 99, 42, 87));
assertEquals("percent [42.0] has been specified twice: [testAgg]", ex.getMessage());
}

public void testExceptionMultipleMethods() throws IOException {
final String illegalAgg = "{\n" +
" \"percentiles\": {\n" +
Expand Down

0 comments on commit 854f698

Please sign in to comment.