github-mirrors/elasticsearch
1
0
Fork 0
mirror of https://github.com/elastic/elasticsearch.git synced 2025-06-28 09:28:55 -04:00
Code Issues Projects Releases Packages Wiki Activity

Handle with illegalArgumentExceptions negative values in HDR percentile aggregations (#116174)

Browse source 
This commit changes to an illegalArgumentException which translate in a bad request (400).
This commit is contained in:
Ignacio Vera 2024-11-05 15:31:08 +01:00 • committed by GitHub
parent 26870ef38d
commit c253a14be8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 57 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

7
docs/changelog/116174.yaml Normal file
View file

@ -0,0 +1,7 @@
pr: 116174
summary: Handle with `illegalArgumentExceptions` negative values in HDR percentile
aggregations
area: Aggregations
type: bug
issues:
- 115777

1
modules/aggregations/build.gradle
View file

@ -49,4 +49,5 @@ dependencies {
tasks.named("yamlRestCompatTestTransform").configure({ task ->
task.skipTest("aggregations/date_agg_per_day_of_week/Date aggregartion per day of week", "week-date behaviour has changed")
task.skipTest("aggregations/time_series/Configure with no synthetic source", "temporary until backport")
task.skipTest("aggregations/percentiles_hdr_metric/Negative values test", "returned exception has changed")
})

2
modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/percentiles_hdr_metric.yml
View file

@ -446,4 +446,4 @@ setup:
- match: { aggregations.percentiles_int.values.75\.0: 101.0615234375 }
- match: { aggregations.percentiles_int.values.95\.0: 151.1240234375 }
- match: { aggregations.percentiles_int.values.99\.0: 151.1240234375 }
- match: { _shards.failures.0.reason.type: array_index_out_of_bounds_exception }
- match: { _shards.failures.0.reason.type: illegal_argument_exception }

12
server/src/main/java/org/elasticsearch/search/aggregations/metrics/AbstractHDRPercentilesAggregator.java
View file

@ -61,7 +61,11 @@ abstract class AbstractHDRPercentilesAggregator extends NumericMetricsAggregator
if (values.advanceExact(doc)) {
final DoubleHistogram state = getExistingOrNewHistogram(bigArrays(), bucket);
for (int i = 0; i < values.docValueCount(); i++) {
state.recordValue(values.nextValue());
final double value = values.nextValue();
if (value < 0) {
throw new IllegalArgumentException("Negative values are not supported by HDR aggregation");
}
state.recordValue(value);
}
}
}
@ -74,8 +78,12 @@ abstract class AbstractHDRPercentilesAggregator extends NumericMetricsAggregator
@Override
public void collect(int doc, long bucket) throws IOException {
if (values.advanceExact(doc)) {
final double value = values.doubleValue();
if (value < 0) {
throw new IllegalArgumentException("Negative values are not supported by HDR aggregation");
}
final DoubleHistogram state = getExistingOrNewHistogram(bigArrays(), bucket);
state.recordValue(values.doubleValue());
state.recordValue(value);
}
}
};

26
server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksAggregatorTests.java
View file

@ -10,6 +10,7 @@
package org.elasticsearch.search.aggregations.metrics;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.NumericDocValuesField;
import org.apache.lucene.document.SortedNumericDocValuesField;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.MultiReader;
@ -29,6 +30,9 @@ import java.io.IOException;
import java.util.Iterator;
import java.util.List;
import static java.util.Collections.singleton;
import static org.hamcrest.Matchers.equalTo;
public class HDRPercentileRanksAggregatorTests extends AggregatorTestCase {
@Override
@ -100,4 +104,26 @@ public class HDRPercentileRanksAggregatorTests extends AggregatorTestCase {
assertThat(e.getMessage(), Matchers.equalTo("[values] must not be an empty array: [my_agg]"));
}
public void testInvalidNegativeNumber() throws IOException {
try (Directory dir = newDirectory(); RandomIndexWriter iw = new RandomIndexWriter(random(), dir)) {
iw.addDocument(singleton(new NumericDocValuesField("number", 60)));
iw.addDocument(singleton(new NumericDocValuesField("number", 40)));
iw.addDocument(singleton(new NumericDocValuesField("number", -20)));
iw.addDocument(singleton(new NumericDocValuesField("number", 10)));
iw.commit();
PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[] { 0.1, 0.5, 12 })
.field("number")
.method(PercentilesMethod.HDR);
MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType("number", NumberFieldMapper.NumberType.LONG);
try (IndexReader reader = iw.getReader()) {
IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> searchAndReduce(reader, new AggTestConfig(aggBuilder, fieldType))
);
assertThat(e.getMessage(), equalTo("Negative values are not supported by HDR aggregation"));
}
}
}
}

12
server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesAggregatorTests.java
View file

@ -164,6 +164,18 @@ public class HDRPercentilesAggregatorTests extends AggregatorTestCase {
assertThat(e.getMessage(), equalTo("Cannot set [compression] because the method has already been configured for HDRHistogram"));
}
public void testInvalidNegativeNumber() {
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
testCase(new MatchAllDocsQuery(), iw -> {
iw.addDocument(singleton(new NumericDocValuesField("number", 60)));
iw.addDocument(singleton(new NumericDocValuesField("number", 40)));
iw.addDocument(singleton(new NumericDocValuesField("number", -20)));
iw.addDocument(singleton(new NumericDocValuesField("number", 10)));
}, hdr -> { fail("Aggregation should have failed due to negative value"); });
});
assertThat(e.getMessage(), equalTo("Negative values are not supported by HDR aggregation"));
}
private void testCase(Query query, CheckedConsumer<RandomIndexWriter, IOException> buildIndex, Consumer<InternalHDRPercentiles> verify)
throws IOException {
MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType("number", NumberFieldMapper.NumberType.LONG);