Fixed an issue where and IndexOutOfBoundsException was thrown when a date_/histogram aggregation was defined on unmapped field and also had a sub aggregation. The root cause there was that in such case, the estimated bucket count was 0, and the code was not designed to handle that well.

Closes #5179
This commit is contained in:
uboness 2014-02-21 02:51:32 +01:00
parent 57fcd761f2
commit 164f7b981e
3 changed files with 42 additions and 5 deletions

View File

@ -73,9 +73,13 @@ public class AggregatorFactories {
ObjectArray<Aggregator> aggregators;
{
aggregators = BigArrays.newObjectArray(estimatedBucketsCount, context.pageCacheRecycler());
// if estimated count is zero, we at least create a single aggregator.
// The estimated count is just an estimation and we can't rely on how it's estimated (that is, an
// estimation of 0 should not imply that we'll end up without any buckets)
long arraySize = estimatedBucketsCount > 0 ? estimatedBucketsCount : 1;
aggregators = BigArrays.newObjectArray(arraySize , context.pageCacheRecycler());
aggregators.set(0, first);
for (long i = 1; i < estimatedBucketsCount; ++i) {
for (long i = 1; i < arraySize; ++i) {
aggregators.set(i, createAndRegisterContextAware(parent.context(), factory, parent, estimatedBucketsCount));
}
}

View File

@ -223,7 +223,7 @@ public class DateHistogramParser implements Aggregator.Parser {
}
if (!(mapper instanceof DateFieldMapper)) {
throw new SearchParseException(context, "date histogram can only be aggregated on date fields but [" + field + "] is not a date field");
throw new SearchParseException(context, "date histogram can only be aggregated on date fields but [" + field + "] is not a date field");
}
IndexFieldData<?> indexFieldData = context.fieldData().getForField(mapper);

View File

@ -25,18 +25,21 @@ import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.search.aggregations.bucket.histogram.Histogram;
import org.elasticsearch.search.aggregations.bucket.missing.Missing;
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.hamcrest.Matchers;
import org.junit.Test;
import java.util.Collection;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.search.aggregations.AggregationBuilders.missing;
import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
import static org.elasticsearch.search.aggregations.AggregationBuilders.*;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.core.IsNull.notNullValue;
/**
*
@ -110,4 +113,34 @@ public class CombiTests extends ElasticsearchIntegrationTest {
}
assertTrue(values.isEmpty());
}
/**
* Some top aggs (eg. date_/histogram) that are executed on unmapped fields, will generate an estimate count of buckets - zero.
* when the sub aggregator is then created, it will take this estimation into account. This used to cause
* and an ArrayIndexOutOfBoundsException...
*/
@Test
public void subAggregationForTopAggregationOnUnmappedField() throws Exception {
prepareCreate("idx").addMapping("type", jsonBuilder()
.startObject()
.startObject("type").startObject("properties")
.startObject("name").field("type", "string").endObject()
.startObject("value").field("type", "integer").endObject()
.endObject().endObject()
.endObject()).execute().actionGet();
ensureSearchable("idx");
SearchResponse searchResponse = client().prepareSearch("idx")
.addAggregation(histogram("values").field("value1").interval(1)
.subAggregation(terms("names").field("name")))
.execute().actionGet();
assertThat(searchResponse.getHits().getTotalHits(), Matchers.equalTo(0l));
Histogram values = searchResponse.getAggregations().get("values");
assertThat(values, notNullValue());
assertThat(values.getBuckets().isEmpty(), is(true));
}
}