From a9ce981b05a1692a71f9b540742f299e03c4ea2c Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Wed, 12 Feb 2020 09:19:57 +0100 Subject: [PATCH 1/6] disallow specifying same percentile values twice --- .../aggregations/metrics/PercentilesAggregationBuilder.java | 6 ++++++ .../search/aggregations/metrics/PercentilesTests.java | 6 ++++++ 2 files changed, 12 insertions(+) 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 851304a4c63ce..6f56019bc5f9b 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 @@ -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: [" + name + "]"); + } + previousPercent = percent; } return sortedPercents; } 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 c18fa664ffd99..5d1d9f1353d32 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 @@ -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" + From 16147e76532c18619d457335f909592488481745 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Fri, 21 Feb 2020 14:23:34 +0100 Subject: [PATCH 2/6] add a release note --- docs/reference/release-notes/8.0.0-alpha1.asciidoc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/reference/release-notes/8.0.0-alpha1.asciidoc b/docs/reference/release-notes/8.0.0-alpha1.asciidoc index 4693221f629cf..21b8020db3eb0 100644 --- a/docs/reference/release-notes/8.0.0-alpha1.asciidoc +++ b/docs/reference/release-notes/8.0.0-alpha1.asciidoc @@ -4,5 +4,12 @@ The changes listed below have been released for the first time in {es} 8.0.0-alpha1. +[[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] From 9e3ad0e6b352bc09c4243c2c4821952ecf8cde7e Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Tue, 25 Feb 2020 11:24:04 +0100 Subject: [PATCH 3/6] adapt to upstream changes --- .../aggregations/metrics/PercentilesAggregationBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6f56019bc5f9b..ca6b13ac5a421 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 @@ -107,7 +107,7 @@ private static double[] validatePercentiles(double[] percents, String aggName) { } if (percent == previousPercent) { - throw new IllegalArgumentException("percent [" + percent + "] has been specified twice: [" + name + "]"); + throw new IllegalArgumentException("percent [" + percent + "] has been specified twice: [" + aggName + "]"); } previousPercent = percent; } From 6250dbc6e238d203378ed4275f02238f31269143 Mon Sep 17 00:00:00 2001 From: lcawl Date: Tue, 25 Feb 2020 10:59:59 -0800 Subject: [PATCH 4/6] [DOCS] Adds breaking changes section for aggregations --- docs/reference/migration/migrate_8_0.asciidoc | 2 ++ .../migration/migrate_8_0/aggregations.asciidoc | 10 ++++++++++ 2 files changed, 12 insertions(+) create mode 100644 docs/reference/migration/migrate_8_0/aggregations.asciidoc diff --git a/docs/reference/migration/migrate_8_0.asciidoc b/docs/reference/migration/migrate_8_0.asciidoc index 4478b678a11f3..bf363ea30f858 100644 --- a/docs/reference/migration/migrate_8_0.asciidoc +++ b/docs/reference/migration/migrate_8_0.asciidoc @@ -11,6 +11,7 @@ See also <> and <>. coming[8.0.0] +* <> * <> * <> * <> @@ -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[] diff --git a/docs/reference/migration/migrate_8_0/aggregations.asciidoc b/docs/reference/migration/migrate_8_0/aggregations.asciidoc new file mode 100644 index 0000000000000..136117f6068a5 --- /dev/null +++ b/docs/reference/migration/migrate_8_0/aggregations.asciidoc @@ -0,0 +1,10 @@ +[float] +[[breaking_80_aggregations_changes]] +=== Analysis changes + +//NOTE: The notable-breaking-changes tagged regions are re-used in the +//Installation and Upgrade Guide + +//tag::notable-breaking-changes[] + +// end::notable-breaking-changes[] \ No newline at end of file From 813e6c9423b49fd077dbe18ab640b64d2900d853 Mon Sep 17 00:00:00 2001 From: lcawl Date: Tue, 25 Feb 2020 14:06:19 -0800 Subject: [PATCH 5/6] [DOCS] Drafts content for aggregations breaking change --- .../migration/migrate_8_0/aggregations.asciidoc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/reference/migration/migrate_8_0/aggregations.asciidoc b/docs/reference/migration/migrate_8_0/aggregations.asciidoc index 136117f6068a5..fd7c3affb15b0 100644 --- a/docs/reference/migration/migrate_8_0/aggregations.asciidoc +++ b/docs/reference/migration/migrate_8_0/aggregations.asciidoc @@ -1,10 +1,17 @@ [float] [[breaking_80_aggregations_changes]] -=== Analysis 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 +<>, +its values must be unique. Otherwise, an exception occurs. // end::notable-breaking-changes[] \ No newline at end of file From 437fcd5a0ebc138d1f50106fba0de92d1da7d3c4 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Wed, 26 Feb 2020 11:26:48 +0100 Subject: [PATCH 6/6] Update docs/reference/release-notes/8.0.0-alpha1.asciidoc Co-Authored-By: Lisa Cawley --- docs/reference/release-notes/8.0.0-alpha1.asciidoc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/reference/release-notes/8.0.0-alpha1.asciidoc b/docs/reference/release-notes/8.0.0-alpha1.asciidoc index 21b8020db3eb0..239ffb0e3f94f 100644 --- a/docs/reference/release-notes/8.0.0-alpha1.asciidoc +++ b/docs/reference/release-notes/8.0.0-alpha1.asciidoc @@ -9,7 +9,6 @@ The changes listed below have been released for the first time in {es} === Breaking changes Aggregations:: -* disallow specifying the same percentile multiple times in percentiles aggregation {pull}52257[#52257] +* Disallow specifying the same percentile multiple times in percentiles aggregation {pull}52257[#52257] coming[8.0.0] -