Percentiles aggregation validation checks for range (#51871)

disallow to specify percentile out of range [0,100]. This also fixes a problem in transform by failing
validation if an invalid percentile configuration is used.
This commit is contained in:
Hendrik Muhs 2020-02-11 17:24:15 +01:00
parent 6fe8f1649b
commit 098380e483
5 changed files with 35 additions and 17 deletions

View File

@ -181,6 +181,12 @@ public class PercentilesAggregationBuilder extends LeafOnly<ValuesSource, Percen
}
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;
}

View File

@ -26,19 +26,21 @@ import org.elasticsearch.plugins.Plugin;
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 @@ public class HDRPercentilesIT extends AbstractNumericTestCase {
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<Double> 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;
}

View File

@ -70,6 +70,15 @@ public class PercentilesTests extends BaseAggregationTestCase<PercentilesAggrega
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" +

View File

@ -26,19 +26,21 @@ import org.elasticsearch.plugins.Plugin;
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 @@ public class TDigestPercentilesIT extends AbstractNumericTestCase {
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<Double> 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;
}

View File

@ -7,11 +7,11 @@ package org.elasticsearch.xpack.analytics.mapper;
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.action.search.SearchResponse;
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 class HistogramPercentileAggregationTests extends ESSingleNodeTestCase {
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();