Skip to content

Commit 39a067a

Browse files
committed
Fix NPE in Derivative Pipeline when current bucket is null
Sequence of events that lead to the NPE: - avg metric returns NaN for buckets - Movavg skips NaN or null buckets, and simply re-uses the existing bucket (e.g. doesn't add a 'movavg' field) - Derivative references Movavg, the bucket resolution returns null because Movavg wasn't added to the bucket, NPE when trying to subtract null values
1 parent 295d33c commit 39a067a

File tree

2 files changed

+35
-5
lines changed

2 files changed

+35
-5
lines changed

core/src/main/java/org/elasticsearch/search/aggregations/pipeline/derivative/DerivativePipelineAggregator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
9292
for (InternalHistogram.Bucket bucket : buckets) {
9393
Long thisBucketKey = resolveBucketKeyAsLong(bucket);
9494
Double thisBucketValue = resolveBucketValue(histo, bucket, bucketsPaths()[0], gapPolicy);
95-
if (lastBucketValue != null) {
95+
if (lastBucketValue != null && thisBucketValue != null) {
9696
double gradient = thisBucketValue - lastBucketValue;
9797
double xDiff = -1;
9898
if (xAxisUnits != null) {

core/src/test/java/org/elasticsearch/search/aggregations/pipeline/DerivativeIT.java

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.elasticsearch.search.aggregations.metrics.sum.Sum;
3434
import org.elasticsearch.search.aggregations.pipeline.BucketHelpers.GapPolicy;
3535
import org.elasticsearch.search.aggregations.pipeline.derivative.Derivative;
36+
import org.elasticsearch.search.aggregations.pipeline.movavg.models.SimpleModel;
3637
import org.elasticsearch.search.aggregations.support.AggregationPath;
3738
import org.elasticsearch.test.ESIntegTestCase;
3839
import org.hamcrest.Matchers;
@@ -43,11 +44,9 @@
4344

4445
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
4546
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
46-
import static org.elasticsearch.search.aggregations.AggregationBuilders.filters;
47-
import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram;
48-
import static org.elasticsearch.search.aggregations.AggregationBuilders.stats;
49-
import static org.elasticsearch.search.aggregations.AggregationBuilders.sum;
47+
import static org.elasticsearch.search.aggregations.AggregationBuilders.*;
5048
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorBuilders.derivative;
49+
import static org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorBuilders.movingAvg;
5150
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
5251
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
5352
import static org.hamcrest.Matchers.closeTo;
@@ -614,6 +613,37 @@ public void testSingleValueAggDerivative_invalidPath() throws Exception {
614613
}
615614
}
616615

616+
public void testAvgMovavgDerivNPE() throws Exception {
617+
createIndex("movavg_npe");
618+
ensureYellow("movavg_npe");
619+
620+
for (int i = 0; i < 10; i++) {
621+
Integer value = i;
622+
if (i == 1 || i == 3) {
623+
value = null;
624+
}
625+
626+
XContentBuilder doc = jsonBuilder()
627+
.startObject()
628+
.field("tick", i)
629+
.field("value", value)
630+
.endObject();
631+
client().prepareIndex("movavg_npe", "type").setSource(doc).get();
632+
}
633+
634+
refresh();
635+
636+
SearchResponse response = client()
637+
.prepareSearch("movavg_npe")
638+
.addAggregation(
639+
histogram("histo").field("tick").interval(1)
640+
.subAggregation(avg("avg").field("value"))
641+
.subAggregation(movingAvg("movavg", "avg").modelBuilder(new SimpleModel.SimpleModelBuilder()).window(3))
642+
.subAggregation(derivative("deriv", "movavg"))).execute().actionGet();
643+
644+
assertSearchResponse(response);
645+
}
646+
617647
private void checkBucketKeyAndDocCount(final String msg, final Histogram.Bucket bucket, final long expectedKey,
618648
final long expectedDocCount) {
619649
assertThat(msg, bucket, notNullValue());

0 commit comments

Comments
 (0)