Skip to content

Commit 1a2f265

Browse files
thomas11jpountz
authored andcommitted
Fix IndexOutOfBoundsException in histograms for NaN doubles (#26787) (#26856)
1 parent 7e0e32a commit 1a2f265

File tree

2 files changed

+27
-3
lines changed

2 files changed

+27
-3
lines changed

core/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,9 @@ protected boolean lessThan(IteratorAndCurrent a, IteratorAndCurrent b) {
269269
do {
270270
final IteratorAndCurrent top = pq.top();
271271

272-
if (top.current.key != key) {
273-
// the key changes, reduce what we already buffered and reset the buffer for current buckets
272+
if (Double.compare(top.current.key, key) != 0) {
273+
// The key changes, reduce what we already buffered and reset the buffer for current buckets.
274+
// Using Double.compare instead of != to handle NaN correctly.
274275
final Bucket reduced = currentBuckets.get(0).reduce(currentBuckets, reduceContext);
275276
if (reduced.getDocCount() >= minDocCount || reduceContext.isFinalReduce() == false) {
276277
reducedBuckets.add(reduced);
@@ -283,7 +284,7 @@ protected boolean lessThan(IteratorAndCurrent a, IteratorAndCurrent b) {
283284

284285
if (top.iterator.hasNext()) {
285286
final Bucket next = top.iterator.next();
286-
assert next.key > top.current.key : "shards must return data sorted by key";
287+
assert Double.compare(next.key, top.current.key) > 0 : "shards must return data sorted by key";
287288
top.current = next;
288289
pq.updateTop();
289290
} else {

core/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogramTests.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@
2121

2222
import org.apache.lucene.util.TestUtil;
2323
import org.elasticsearch.search.DocValueFormat;
24+
import org.elasticsearch.search.aggregations.InternalAggregation;
2425
import org.elasticsearch.search.aggregations.InternalAggregations;
2526
import org.elasticsearch.search.aggregations.InternalMultiBucketAggregationTestCase;
2627
import org.elasticsearch.search.aggregations.ParsedMultiBucketAggregation;
2728
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
2829

2930
import java.util.ArrayList;
31+
import java.util.Arrays;
3032
import java.util.List;
3133
import java.util.Map;
3234

@@ -59,6 +61,27 @@ protected InternalHistogram createTestInstance(String name,
5961
return new InternalHistogram(name, buckets, order, 1, null, format, keyed, pipelineAggregators, metaData);
6062
}
6163

64+
// issue 26787
65+
public void testHandlesNaN() {
66+
InternalHistogram histogram = createTestInstance();
67+
InternalHistogram histogram2 = createTestInstance();
68+
List<InternalHistogram.Bucket> buckets = histogram.getBuckets();
69+
if (buckets == null || buckets.isEmpty()) {
70+
return;
71+
}
72+
73+
// Set the key of one bucket to NaN. Must be the last bucket because NaN is greater than everything else.
74+
List<InternalHistogram.Bucket> newBuckets = new ArrayList<>(buckets.size());
75+
if (buckets.size() > 1) {
76+
newBuckets.addAll(buckets.subList(0, buckets.size() - 1));
77+
}
78+
InternalHistogram.Bucket b = buckets.get(buckets.size() - 1);
79+
newBuckets.add(new InternalHistogram.Bucket(Double.NaN, b.docCount, keyed, b.format, b.aggregations));
80+
81+
InternalHistogram newHistogram = histogram.create(newBuckets);
82+
newHistogram.doReduce(Arrays.asList(newHistogram, histogram2), new InternalAggregation.ReduceContext(null, null, false));
83+
}
84+
6285
@Override
6386
protected Class<? extends ParsedMultiBucketAggregation> implementationClass() {
6487
return ParsedHistogram.class;

0 commit comments

Comments
 (0)