Pass resolved extended bounds to unmapped histogram aggregator
Previous to this change the unresolved extended bounds was passed into the histogram aggregator which meant extendedbounds.min and extendedbounds.max was passed through as null. This had two effects on the histogram aggregator: 1. If the histogram aggregator was unmapped across all shards, the reduce phase would not add buckets for the extended bounds and the response would contain zero buckets 2. If the histogram aggregator was not unmapped in some shards, the reduce phase might sometimes chose to reduce based on the unmapped shard response and therefore the extended bounds would be ignored. This change resolves the extended bounds in the unmapped case and solves the above two issues. Closes #19009
This commit is contained in:
parent
cb0824e957
commit
108ba23073
|
@ -69,9 +69,7 @@ public abstract class AbstractHistogramAggregatorFactory<AF extends AbstractHist
|
|||
@Override
|
||||
protected Aggregator createUnmapped(Aggregator parent, List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData)
|
||||
throws IOException {
|
||||
Rounding rounding = createRounding();
|
||||
return new HistogramAggregator(name, factories, rounding, order, keyed, minDocCount, extendedBounds, null, config.format(),
|
||||
histogramFactory, context, parent, pipelineAggregators, metaData);
|
||||
return createAggregator(null, parent, pipelineAggregators, metaData);
|
||||
}
|
||||
|
||||
protected Rounding createRounding() {
|
||||
|
@ -92,6 +90,11 @@ public abstract class AbstractHistogramAggregatorFactory<AF extends AbstractHist
|
|||
if (collectsFromSingleBucket == false) {
|
||||
return asMultiBucketAggregator(this, context, parent);
|
||||
}
|
||||
return createAggregator(valuesSource, parent, pipelineAggregators, metaData);
|
||||
}
|
||||
|
||||
private Aggregator createAggregator(ValuesSource.Numeric valuesSource, Aggregator parent, List<PipelineAggregator> pipelineAggregators,
|
||||
Map<String, Object> metaData) throws IOException {
|
||||
Rounding rounding = createRounding();
|
||||
// we need to round the bounds given by the user and we have to do it
|
||||
// for every aggregator we create
|
||||
|
|
|
@ -21,6 +21,7 @@ package org.elasticsearch.messy.tests;
|
|||
import com.carrotsearch.hppc.LongHashSet;
|
||||
import org.elasticsearch.action.index.IndexRequestBuilder;
|
||||
import org.elasticsearch.action.search.SearchResponse;
|
||||
import org.elasticsearch.index.query.QueryBuilders;
|
||||
import org.elasticsearch.plugins.Plugin;
|
||||
import org.elasticsearch.script.Script;
|
||||
import org.elasticsearch.script.groovy.GroovyPlugin;
|
||||
|
@ -825,6 +826,83 @@ public class HistogramTests extends ESIntegTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
public void testEmptyWithExtendedBounds() throws Exception {
|
||||
int lastDataBucketKey = (numValueBuckets - 1) * interval;
|
||||
|
||||
// randomizing the number of buckets on the min bound
|
||||
// (can sometimes fall within the data range, but more frequently will fall before the data range)
|
||||
int addedBucketsLeft = randomIntBetween(0, numValueBuckets);
|
||||
long boundsMinKey = addedBucketsLeft * interval;
|
||||
if (frequently()) {
|
||||
boundsMinKey = -boundsMinKey;
|
||||
} else {
|
||||
addedBucketsLeft = 0;
|
||||
}
|
||||
long boundsMin = boundsMinKey + randomIntBetween(0, interval - 1);
|
||||
|
||||
// randomizing the number of buckets on the max bound
|
||||
// (can sometimes fall within the data range, but more frequently will fall after the data range)
|
||||
int addedBucketsRight = randomIntBetween(0, numValueBuckets);
|
||||
long boundsMaxKeyDelta = addedBucketsRight * interval;
|
||||
if (rarely()) {
|
||||
addedBucketsRight = 0;
|
||||
boundsMaxKeyDelta = -boundsMaxKeyDelta;
|
||||
}
|
||||
long boundsMaxKey = lastDataBucketKey + boundsMaxKeyDelta;
|
||||
long boundsMax = boundsMaxKey + randomIntBetween(0, interval - 1);
|
||||
|
||||
|
||||
// it could be that the random bounds.min we chose ended up greater than bounds.max - this should cause an
|
||||
// error
|
||||
boolean invalidBoundsError = boundsMin > boundsMax;
|
||||
|
||||
// constructing the newly expected bucket list
|
||||
int bucketsCount = numValueBuckets + addedBucketsLeft + addedBucketsRight;
|
||||
long[] extendedValueCounts = new long[bucketsCount];
|
||||
System.arraycopy(valueCounts, 0, extendedValueCounts, addedBucketsLeft, valueCounts.length);
|
||||
|
||||
SearchResponse response = null;
|
||||
try {
|
||||
response = client().prepareSearch("idx")
|
||||
.setQuery(QueryBuilders.termQuery("foo", "bar"))
|
||||
.addAggregation(histogram("histo")
|
||||
.field(SINGLE_VALUED_FIELD_NAME)
|
||||
.interval(interval)
|
||||
.minDocCount(0)
|
||||
.extendedBounds(new ExtendedBounds(boundsMin, boundsMax)))
|
||||
.execute().actionGet();
|
||||
|
||||
if (invalidBoundsError) {
|
||||
fail("Expected an exception to be thrown when bounds.min is greater than bounds.max");
|
||||
return;
|
||||
}
|
||||
|
||||
} catch (Exception e) {
|
||||
if (invalidBoundsError) {
|
||||
// expected
|
||||
return;
|
||||
} else {
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
assertSearchResponse(response);
|
||||
|
||||
Histogram histo = response.getAggregations().get("histo");
|
||||
assertThat(histo, notNullValue());
|
||||
assertThat(histo.getName(), equalTo("histo"));
|
||||
List<? extends Bucket> buckets = histo.getBuckets();
|
||||
assertThat(buckets.size(), equalTo(bucketsCount));
|
||||
|
||||
long key = Math.min(boundsMinKey, 0);
|
||||
for (int i = 0; i < bucketsCount; i++) {
|
||||
Histogram.Bucket bucket = buckets.get(i);
|
||||
assertThat(bucket, notNullValue());
|
||||
assertThat(((Number) bucket.getKey()).longValue(), equalTo(key));
|
||||
assertThat(bucket.getDocCount(), equalTo(0L));
|
||||
key += interval;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* see issue #9634, negative interval in histogram should raise exception
|
||||
*/
|
||||
|
|
Loading…
Reference in New Issue