diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java index 6c0ce35b804..f68cf0abe5a 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java @@ -49,7 +49,7 @@ public abstract class AbstractObjectParser /** * Declares named objects in the style of aggregations. These are named * inside and object like this: - * + * *
      * 
      * {
@@ -62,7 +62,7 @@ public abstract class AbstractObjectParser
      * }
      * 
      * 
- * + * * Unlike the other version of this method, "ordered" mode (arrays of * objects) is not supported. * @@ -82,7 +82,7 @@ public abstract class AbstractObjectParser /** * Declares named objects in the style of highlighting's field element. * These are usually named inside and object like this: - * + * *
      * 
      * {
@@ -96,9 +96,9 @@ public abstract class AbstractObjectParser
      * }
      * 
      * 
- * + * * but, when order is important, some may be written this way: - * + * *
      * 
      * {
@@ -112,7 +112,7 @@ public abstract class AbstractObjectParser
      * }
      * 
      * 
- * + * * This is because json doesn't enforce ordering. Elasticsearch reads it in * the order sent but tools that generate json are free to put object * members in an unordered Map, jumbling them. Thus, if you care about order @@ -134,6 +134,8 @@ public abstract class AbstractObjectParser public abstract void declareNamedObjects(BiConsumer> consumer, NamedObjectParser namedObjectParser, Consumer orderedModeCallback, ParseField parseField); + public abstract String getName(); + public void declareField(BiConsumer consumer, CheckedFunction parser, ParseField parseField, ValueType type) { if (parser == null) { diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java index 2b478b9018a..53b809ba6ed 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java @@ -295,6 +295,10 @@ public final class ConstructingObjectParser extends AbstractObje } } + public String getName() { + return objectParser.getName(); + } + private Consumer wrapOrderedModeCallBack(Consumer callback) { return (target) -> { if (target.targetObject != null) { diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/AggregationBuilders.java b/core/src/main/java/org/elasticsearch/search/aggregations/AggregationBuilders.java index fcd3c7f01ea..26d8bb1a1bd 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/AggregationBuilders.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/AggregationBuilders.java @@ -247,15 +247,15 @@ public class AggregationBuilders { return new SignificantTermsAggregationBuilder(name, null); } - + /** * Create a new {@link SignificantTextAggregationBuilder} aggregation with the given name and text field name */ public static SignificantTextAggregationBuilder significantText(String name, String fieldName) { return new SignificantTextAggregationBuilder(name, fieldName); - } - - + } + + /** * Create a new {@link DateHistogramAggregationBuilder} aggregation with the given * name. @@ -304,8 +304,8 @@ public class AggregationBuilders { /** * Create a new {@link PercentileRanks} aggregation with the given name. */ - public static PercentileRanksAggregationBuilder percentileRanks(String name) { - return new PercentileRanksAggregationBuilder(name); + public static PercentileRanksAggregationBuilder percentileRanks(String name, double[] values) { + return new PercentileRanksAggregationBuilder(name, values); } /** diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/PercentileRanksAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/PercentileRanksAggregationBuilder.java index ea8e7648b51..41dac8d4961 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/PercentileRanksAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/PercentileRanksAggregationBuilder.java @@ -22,6 +22,8 @@ package org.elasticsearch.search.aggregations.metrics.percentiles; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ContextParser; import org.elasticsearch.common.xcontent.ObjectParser; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -42,7 +44,11 @@ import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; import java.util.Arrays; +import java.util.List; import java.util.Objects; +import java.util.function.BiConsumer; + +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; public class PercentileRanksAggregationBuilder extends LeafOnly { public static final String NAME = PercentileRanks.TYPE_NAME; @@ -53,7 +59,7 @@ public class PercentileRanksAggregationBuilder extends LeafOnly TDIGEST_OPTIONS_PARSER = + private static final ObjectParser TDIGEST_OPTIONS_PARSER = new ObjectParser<>(PercentilesMethod.TDIGEST.getParseField().getPreferredName(), TDigestOptions::new); static { TDIGEST_OPTIONS_PARSER.declareDouble((opts, compression) -> opts.compression = compression, new ParseField("compression")); @@ -63,22 +69,22 @@ public class PercentileRanksAggregationBuilder extends LeafOnly HDR_OPTIONS_PARSER = + private static final ObjectParser HDR_OPTIONS_PARSER = new ObjectParser<>(PercentilesMethod.HDR.getParseField().getPreferredName(), HDROptions::new); static { HDR_OPTIONS_PARSER.declareInt((opts, numberOfSigDigits) -> opts.numberOfSigDigits = numberOfSigDigits, new ParseField("number_of_significant_value_digits")); } - private static final ObjectParser PARSER; + // The builder requires two parameters for the constructor: aggregation name and values array. The + // agg name is supplied externally via the Parser's context (as a String), while the values array + // is parsed from the request and supplied to the ConstructingObjectParser as a ctor argument + private static final ConstructingObjectParser PARSER; static { - PARSER = new ObjectParser<>(PercentileRanksAggregationBuilder.NAME); + PARSER = new ConstructingObjectParser<>(PercentileRanksAggregationBuilder.NAME, false, + (a, context) -> new PercentileRanksAggregationBuilder(context, (List) a[0])); ValuesSourceParserHelper.declareNumericFields(PARSER, true, false, false); - - PARSER.declareDoubleArray( - (b, v) -> b.values(v.stream().mapToDouble(Double::doubleValue).toArray()), - VALUES_FIELD); - + PARSER.declareDoubleArray(constructorArg(), VALUES_FIELD); PARSER.declareBoolean(PercentileRanksAggregationBuilder::keyed, PercentilesAggregationBuilder.KEYED_FIELD); PARSER.declareField((b, v) -> { @@ -97,7 +103,8 @@ public class PercentileRanksAggregationBuilder extends LeafOnly values) { + this(name, values.stream().mapToDouble(Double::doubleValue).toArray()); + } + + public PercentileRanksAggregationBuilder(String name, double[] values) { super(name, ValuesSourceType.NUMERIC, ValueType.NUMERIC); + if (values == null) { + throw new IllegalArgumentException("[values] must not be null: [" + name + "]"); + } + if (values.length == 0) { + throw new IllegalArgumentException("[values] must not be an empty array: [" + name + "]"); + } + double[] sortedValues = Arrays.copyOf(values, values.length); + Arrays.sort(sortedValues); + this.values = sortedValues; } /** @@ -131,19 +151,6 @@ public class PercentileRanksAggregationBuilder extends LeafOnly, Void> objectParser, + public static void declareAnyFields( + AbstractObjectParser, T> objectParser, boolean scriptable, boolean formattable) { declareFields(objectParser, scriptable, formattable, false, null); } - public static void declareNumericFields( - ObjectParser, Void> objectParser, + public static void declareNumericFields( + AbstractObjectParser, T> objectParser, boolean scriptable, boolean formattable, boolean timezoneAware) { declareFields(objectParser, scriptable, formattable, timezoneAware, ValueType.NUMERIC); } - public static void declareBytesFields( - ObjectParser, Void> objectParser, + public static void declareBytesFields( + AbstractObjectParser, T> objectParser, boolean scriptable, boolean formattable) { declareFields(objectParser, scriptable, formattable, false, ValueType.STRING); } - public static void declareGeoFields( - ObjectParser, Void> objectParser, + public static void declareGeoFields( + AbstractObjectParser, T> objectParser, boolean scriptable, boolean formattable) { declareFields(objectParser, scriptable, formattable, false, ValueType.GEOPOINT); } - private static void declareFields( - ObjectParser, Void> objectParser, + private static void declareFields( + AbstractObjectParser, T> objectParser, boolean scriptable, boolean formattable, boolean timezoneAware, ValueType targetValueType) { @@ -99,4 +101,6 @@ public final class ValuesSourceParserHelper { } } + + } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java index 586af22755c..cf994052131 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksIT.java @@ -128,8 +128,9 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .interval(1L) .minDocCount(0) .subAggregation( - percentileRanks("percentile_ranks").field("value").method(PercentilesMethod.HDR) - .numberOfSignificantValueDigits(sigDigits).values(10, 15))) + percentileRanks("percentile_ranks", new double[]{10, 15}) + .field("value").method(PercentilesMethod.HDR) + .numberOfSignificantValueDigits(sigDigits))) .execute().actionGet(); assertThat(searchResponse.getHits().getTotalHits(), equalTo(2L)); @@ -152,8 +153,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx_unmapped") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits) - .field("value").values(0, 10, 15, 100)) + percentileRanks("percentile_ranks", new double[]{0, 10, 15, 100}) + .method(PercentilesMethod.HDR) + .numberOfSignificantValueDigits(sigDigits) + .field("value")) .execute().actionGet(); assertThat(searchResponse.getHits().getTotalHits(), equalTo(0L)); @@ -175,8 +178,8 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits) - .field("value").values(pcts)) + percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits) + .field("value")) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -185,6 +188,36 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { assertConsistent(pcts, values, minValue, maxValue, sigDigits); } + public void testNullValuesField() throws Exception { + int sigDigits = randomSignificantDigits(); + final double[] pcts = null; + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client() + .prepareSearch("idx") + .setQuery(matchAllQuery()) + .addAggregation( + percentileRanks("percentile_ranks", pcts) + .method(PercentilesMethod.HDR) + .numberOfSignificantValueDigits(sigDigits) + .field("value")) + .execute().actionGet()); + assertThat(e.getMessage(), equalTo("[values] must not be null: [percentile_ranks]")); + } + + public void testEmptyValuesField() throws Exception { + int sigDigits = randomSignificantDigits(); + final double[] pcts = new double[0]; + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client() + .prepareSearch("idx") + .setQuery(matchAllQuery()) + .addAggregation( + percentileRanks("percentile_ranks", pcts) + .method(PercentilesMethod.HDR) + .numberOfSignificantValueDigits(sigDigits) + .field("value")) + .execute().actionGet()); + assertThat(e.getMessage(), equalTo("[values] must not be an empty array: [percentile_ranks]")); + } + @Override public void testSingleValuedFieldGetProperty() throws Exception { int sigDigits = randomSignificantDigits(); @@ -194,8 +227,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( global("global").subAggregation( - percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits) - .field("value").values(pcts))) + percentileRanks("percentile_ranks", pcts) + .method(PercentilesMethod.HDR) + .numberOfSignificantValueDigits(sigDigits) + .field("value"))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -221,8 +256,8 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits) - .field("value").values(pcts)) + percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits) + .field("value")) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -239,8 +274,8 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx", "idx_unmapped") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits) - .field("value").values(pcts)) + percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits) + .field("value")) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -257,12 +292,11 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks") + percentileRanks("percentile_ranks", pcts) .method(PercentilesMethod.HDR) .numberOfSignificantValueDigits(sigDigits) .field("value") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap())) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -281,12 +315,11 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks") + percentileRanks("percentile_ranks", pcts) .method(PercentilesMethod.HDR) .numberOfSignificantValueDigits(sigDigits) .field("value") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params)) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -303,8 +336,8 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks").method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits) - .field("values").values(pcts)) + percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.HDR).numberOfSignificantValueDigits(sigDigits) + .field("values")) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -321,12 +354,11 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks") + percentileRanks("percentile_ranks", pcts) .method(PercentilesMethod.HDR) .numberOfSignificantValueDigits(sigDigits) .field("values") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap())) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -342,12 +374,11 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks") + percentileRanks("percentile_ranks", pcts) .method(PercentilesMethod.HDR) .numberOfSignificantValueDigits(sigDigits) .field("values") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "20 - _value", emptyMap())) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "20 - _value", emptyMap()))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -366,12 +397,11 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks") + percentileRanks("percentile_ranks", pcts) .method(PercentilesMethod.HDR) .numberOfSignificantValueDigits(sigDigits) .field("values") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params)) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -388,11 +418,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks") + percentileRanks("percentile_ranks", pcts) .method(PercentilesMethod.HDR) .numberOfSignificantValueDigits(sigDigits) - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "doc['value'].value", emptyMap())) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "doc['value'].value", emptyMap()))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -414,11 +443,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks") + percentileRanks("percentile_ranks", pcts) .method(PercentilesMethod.HDR) .numberOfSignificantValueDigits(sigDigits) - .script(script) - .values(pcts)) + .script(script)) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -438,11 +466,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks") + percentileRanks("percentile_ranks", pcts) .method(PercentilesMethod.HDR) .numberOfSignificantValueDigits(sigDigits) - .script(script) - .values(pcts)) + .script(script)) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -461,11 +488,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .prepareSearch("idx") .setQuery(matchAllQuery()) .addAggregation( - percentileRanks("percentile_ranks") + percentileRanks("percentile_ranks", pcts) .method(PercentilesMethod.HDR) .numberOfSignificantValueDigits(sigDigits) - .script(script) - .values(pcts)) + .script(script)) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -483,8 +509,8 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { .addAggregation( histogram("histo").field("value").interval(2L) .subAggregation( - percentileRanks("percentile_ranks").field("value").method(PercentilesMethod.HDR) - .numberOfSignificantValueDigits(sigDigits).values(99)) + percentileRanks("percentile_ranks", new double[]{99}).field("value").method(PercentilesMethod.HDR) + .numberOfSignificantValueDigits(sigDigits)) .order(BucketOrder.aggregation("percentile_ranks", "99", asc))).execute().actionGet(); assertHitCount(searchResponse, 10); @@ -508,7 +534,7 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { SearchResponse searchResponse = client().prepareSearch("idx").setQuery(matchAllQuery()) .addAggregation(terms("terms").field("value").order(BucketOrder.compound(BucketOrder.aggregation("filter>ranks.99", true))) .subAggregation(filter("filter", termQuery("value", 100)) - .subAggregation(percentileRanks("ranks").method(PercentilesMethod.HDR).values(99).field("value")))) + .subAggregation(percentileRanks("ranks", new double[]{99}).method(PercentilesMethod.HDR).field("value")))) .get(); assertHitCount(searchResponse, 10); @@ -553,8 +579,10 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { // Test that a request using a script does not get cached SearchResponse r = client() - .prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo").method(PercentilesMethod.HDR).field("d") - .values(50.0).script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) + .prepareSearch("cache_test_idx").setSize(0) + .addAggregation(percentileRanks("foo", new double[]{50.0}) + .method(PercentilesMethod.HDR).field("d") + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) .get(); assertSearchResponse(r); @@ -566,7 +594,7 @@ public class HDRPercentileRanksIT extends AbstractNumericTestCase { // To make sure that the cache is working test that a request not using // a script is cached r = client().prepareSearch("cache_test_idx").setSize(0) - .addAggregation(percentileRanks("foo").method(PercentilesMethod.HDR).field("d").values(50.0)).get(); + .addAggregation(percentileRanks("foo", new double[]{50.0}).method(PercentilesMethod.HDR).field("d")).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksTests.java index b78a67e9511..a678f69f19b 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.search.aggregations.metrics; -import org.elasticsearch.script.Script; import org.elasticsearch.search.aggregations.BaseAggregationTestCase; import org.elasticsearch.search.aggregations.metrics.percentiles.PercentileRanksAggregationBuilder; @@ -27,16 +26,16 @@ public class PercentileRanksTests extends BaseAggregationTestCase client() + .prepareSearch("idx") + .setQuery(matchAllQuery()) + .addAggregation( + percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.TDIGEST).field("value")) + .execute().actionGet()); + assertThat(e.getMessage(), equalTo("[values] must not be null: [percentile_ranks]")); + } + + public void testEmptyValuesField() throws Exception { + final double[] pcts = new double[0]; + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client() + .prepareSearch("idx") + .setQuery(matchAllQuery()) + .addAggregation( + percentileRanks("percentile_ranks", pcts).method(PercentilesMethod.TDIGEST).field("value")) + .execute().actionGet()); + assertThat(e.getMessage(), equalTo("[values] must not be an empty array: [percentile_ranks]")); + } + @Override public void testUnmapped() throws Exception { SearchResponse searchResponse = client().prepareSearch("idx_unmapped") .setQuery(matchAllQuery()) - .addAggregation(randomCompression(percentileRanks("percentile_ranks")) - .field("value") - .values(0, 10, 15, 100)) + .addAggregation(randomCompression(percentileRanks("percentile_ranks", new double[]{0, 10, 15, 100})) + .field("value")) .execute().actionGet(); assertThat(searchResponse.getHits().getTotalHits(), equalTo(0L)); @@ -166,9 +186,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { final double[] pcts = randomPercents(minValue, maxValue); SearchResponse searchResponse = client().prepareSearch("idx") .setQuery(matchAllQuery()) - .addAggregation(randomCompression(percentileRanks("percentile_ranks")) - .field("value") - .values(pcts)) + .addAggregation(randomCompression(percentileRanks("percentile_ranks", pcts)) + .field("value")) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -185,7 +204,7 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( global("global").subAggregation( - randomCompression(percentileRanks("percentile_ranks")).field("value").values(pcts))).execute() + randomCompression(percentileRanks("percentile_ranks", pcts)).field("value"))).execute() .actionGet(); assertHitCount(searchResponse, 10); @@ -207,9 +226,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { final double[] pcts = new double[] {minValue - 1, maxValue + 1}; SearchResponse searchResponse = client().prepareSearch("idx") .setQuery(matchAllQuery()) - .addAggregation(randomCompression(percentileRanks("percentile_ranks")) - .field("value") - .values(pcts)) + .addAggregation(randomCompression(percentileRanks("percentile_ranks", pcts)) + .field("value")) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -223,9 +241,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { final double[] pcts = randomPercents(minValue, maxValue); SearchResponse searchResponse = client().prepareSearch("idx", "idx_unmapped") .setQuery(matchAllQuery()) - .addAggregation(randomCompression(percentileRanks("percentile_ranks")) - .field("value") - .values(pcts)) + .addAggregation(randomCompression(percentileRanks("percentile_ranks", pcts)) + .field("value")) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -241,10 +258,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( randomCompression( - percentileRanks("percentile_ranks")) + percentileRanks("percentile_ranks", pcts)) .field("value") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap())) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -262,10 +278,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( randomCompression( - percentileRanks("percentile_ranks")) + percentileRanks("percentile_ranks", pcts)) .field("value") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params)) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -279,9 +294,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { final double[] pcts = randomPercents(minValues, maxValues); SearchResponse searchResponse = client().prepareSearch("idx") .setQuery(matchAllQuery()) - .addAggregation(randomCompression(percentileRanks("percentile_ranks")) - .field("values") - .values(pcts)) + .addAggregation(randomCompression(percentileRanks("percentile_ranks", pcts)) + .field("values")) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -297,10 +311,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( randomCompression( - percentileRanks("percentile_ranks")) + percentileRanks("percentile_ranks", pcts)) .field("values") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap())) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -315,10 +328,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( randomCompression( - percentileRanks("percentile_ranks")) + percentileRanks("percentile_ranks", pcts)) .field("values") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value * -1", emptyMap())) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value * -1", emptyMap()))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -336,10 +348,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( randomCompression( - percentileRanks("percentile_ranks")) + percentileRanks("percentile_ranks", pcts)) .field("values") - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params)) - .values(pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - dec", params))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -355,9 +366,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( randomCompression( - percentileRanks("percentile_ranks")) - .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "doc['value'].value", emptyMap())) - .values(pcts)) + percentileRanks("percentile_ranks", pcts)) + .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "doc['value'].value", emptyMap()))) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -378,9 +388,7 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( randomCompression( - percentileRanks("percentile_ranks")) - .script(script) - .values(pcts)) + percentileRanks("percentile_ranks", pcts)).script(script)) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -397,9 +405,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( randomCompression( - percentileRanks("percentile_ranks")) - .script(script) - .values(pcts)) + percentileRanks("percentile_ranks", pcts)) + .script(script)) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -417,9 +424,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( randomCompression( - percentileRanks("percentile_ranks")) - .script(script) - .values(pcts)) + percentileRanks("percentile_ranks", pcts)) + .script(script)) .execute().actionGet(); assertHitCount(searchResponse, 10); @@ -434,7 +440,7 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .setQuery(matchAllQuery()) .addAggregation( histogram("histo").field("value").interval(2L) - .subAggregation(randomCompression(percentileRanks("percentile_ranks").field("value").values(99))) + .subAggregation(randomCompression(percentileRanks("percentile_ranks", new double[]{99}).field("value"))) .order(BucketOrder.aggregation("percentile_ranks", "99", asc))) .execute().actionGet(); @@ -459,7 +465,8 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { SearchResponse searchResponse = client().prepareSearch("idx").setQuery(matchAllQuery()) .addAggregation(terms("terms").field("value").order(BucketOrder.compound(BucketOrder.aggregation("filter>ranks.99", true))) .subAggregation(filter("filter", termQuery("value", 100)) - .subAggregation(percentileRanks("ranks").method(PercentilesMethod.TDIGEST).values(99).field("value")))) + .subAggregation(percentileRanks("ranks", new double[]{99}) + .method(PercentilesMethod.TDIGEST).field("value")))) .get(); assertHitCount(searchResponse, 10); @@ -503,7 +510,9 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { .getMissCount(), equalTo(0L)); // Test that a request using a script does not get cached - SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo").field("d").values(50.0) + SearchResponse r = client().prepareSearch("cache_test_idx").setSize(0) + .addAggregation(percentileRanks("foo", new double[]{50.0}) + .field("d") .script(new Script(ScriptType.INLINE, AggregationTestScriptsPlugin.NAME, "_value - 1", emptyMap()))).get(); assertSearchResponse(r); @@ -514,7 +523,7 @@ public class TDigestPercentileRanksIT extends AbstractNumericTestCase { // To make sure that the cache is working test that a request not using // a script is cached - r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo").field("d").values(50.0)).get(); + r = client().prepareSearch("cache_test_idx").setSize(0).addAggregation(percentileRanks("foo", new double[]{50.0}).field("d")).get(); assertSearchResponse(r); assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/HDRPercentileRanksAggregatorTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/HDRPercentileRanksAggregatorTests.java index b0a3008d21f..505480b2161 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/HDRPercentileRanksAggregatorTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/HDRPercentileRanksAggregatorTests.java @@ -40,13 +40,14 @@ import org.hamcrest.Matchers; import java.io.IOException; import java.util.Iterator; +import static org.hamcrest.core.IsEqual.equalTo; + public class HDRPercentileRanksAggregatorTests extends AggregatorTestCase { public void testEmpty() throws IOException { - PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg") + PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[]{0.5}) .field("field") - .method(PercentilesMethod.HDR) - .values(0.5); + .method(PercentilesMethod.HDR); MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE); fieldType.setName("field"); try (IndexReader reader = new MultiReader()) { @@ -67,10 +68,9 @@ public class HDRPercentileRanksAggregatorTests extends AggregatorTestCase { w.addDocument(doc); } - PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg") + PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[]{0.1, 0.5, 12}) .field("field") - .method(PercentilesMethod.HDR) - .values(0.1, 0.5, 12); + .method(PercentilesMethod.HDR); MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE); fieldType.setName("field"); try (IndexReader reader = w.getReader()) { @@ -91,4 +91,17 @@ public class HDRPercentileRanksAggregatorTests extends AggregatorTestCase { } } } + + public void testNullValues() throws IOException { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new PercentileRanksAggregationBuilder("my_agg", null).field("field").method(PercentilesMethod.HDR)); + assertThat(e.getMessage(), Matchers.equalTo("[values] must not be null: [my_agg]")); + } + + public void testEmptyValues() throws IOException { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new PercentileRanksAggregationBuilder("my_agg", new double[0]).field("field").method(PercentilesMethod.HDR)); + + assertThat(e.getMessage(), Matchers.equalTo("[values] must not be an empty array: [my_agg]")); + } } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/tdigest/TDigestPercentileRanksAggregatorTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/tdigest/TDigestPercentileRanksAggregatorTests.java index 0f786a820e1..9bf7fdf0486 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/tdigest/TDigestPercentileRanksAggregatorTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/tdigest/TDigestPercentileRanksAggregatorTests.java @@ -40,13 +40,14 @@ import org.hamcrest.Matchers; import java.io.IOException; import java.util.Iterator; +import static org.hamcrest.core.IsEqual.equalTo; + public class TDigestPercentileRanksAggregatorTests extends AggregatorTestCase { public void testEmpty() throws IOException { - PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg") + PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[]{0.5}) .field("field") - .method(PercentilesMethod.TDIGEST) - .values(0.5); + .method(PercentilesMethod.TDIGEST); MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE); fieldType.setName("field"); try (IndexReader reader = new MultiReader()) { @@ -67,10 +68,9 @@ public class TDigestPercentileRanksAggregatorTests extends AggregatorTestCase { w.addDocument(doc); } - PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg") + PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[]{0.1, 0.5, 12}) .field("field") - .method(PercentilesMethod.TDIGEST) - .values(0.1, 0.5, 12); + .method(PercentilesMethod.TDIGEST); MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE); fieldType.setName("field"); try (IndexReader reader = w.getReader()) { @@ -95,4 +95,17 @@ public class TDigestPercentileRanksAggregatorTests extends AggregatorTestCase { } } } + + public void testNullValues() throws IOException { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new PercentileRanksAggregationBuilder("my_agg", null).field("field").method(PercentilesMethod.TDIGEST)); + assertThat(e.getMessage(), Matchers.equalTo("[values] must not be null: [my_agg]")); + } + + public void testEmptyValues() throws IOException { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> new PercentileRanksAggregationBuilder("my_agg", new double[0]).field("field").method(PercentilesMethod.TDIGEST)); + + assertThat(e.getMessage(), Matchers.equalTo("[values] must not be an empty array: [my_agg]")); + } } diff --git a/docs/reference/migration/migrate_6_0/java.asciidoc b/docs/reference/migration/migrate_6_0/java.asciidoc index 937538f45e2..b706b4abb14 100644 --- a/docs/reference/migration/migrate_6_0/java.asciidoc +++ b/docs/reference/migration/migrate_6_0/java.asciidoc @@ -56,3 +56,9 @@ The `org.elasticsearch.search.aggregations.bucket.terms.support` package was rem `org.elasticsearch.search.aggregations.bucket.terms`. The filter aggregation classes were moved to `org.elasticsearch.search.aggregations.bucket.filter` + +=== Constructor for `PercentileRanksAggregationBuilder` has changed + +It is now required to include the desired ranks as a non-null, non-empty array of doubles to the builder's constructor, +rather than configuring them via a setter on the builder. The setter method `values()` has correspondingly +been removed. \ No newline at end of file