Fixes array out of bounds for value count agg (#26038)

https://github.com/elastic/elasticsearch/pull/17379 fixed many metric aggs so that if the parent aggregation does not collect any documents an empty bucket value is returned instead of an ArrayOutOfBoundsException being thrown. Unfortunately the value count aggregation was mised from this fix.

This change applies this fix from #17379 for the value count aggregation.
This commit is contained in:
Colin Goodheart-Smithe 2017-08-03 10:19:14 +01:00 committed by GitHub
parent d8414ffa29
commit 5f1634dff4
2 changed files with 41 additions and 5 deletions

View File

@ -83,7 +83,7 @@ public class ValueCountAggregator extends NumericMetricsAggregator.SingleValue {
@Override @Override
public double metric(long owningBucketOrd) { public double metric(long owningBucketOrd) {
return valuesSource == null ? 0 : counts.get(owningBucketOrd); return (valuesSource == null || owningBucketOrd >= counts.size()) ? 0 : counts.get(owningBucketOrd);
} }
@Override @Override

View File

@ -18,24 +18,31 @@
*/ */
package org.elasticsearch.search.aggregations.metrics; package org.elasticsearch.search.aggregations.metrics;
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.script.Script; import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptType; import org.elasticsearch.script.ScriptType;
import org.elasticsearch.search.aggregations.BucketOrder;
import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.InternalAggregation;
import org.elasticsearch.search.aggregations.bucket.filter.Filter;
import org.elasticsearch.search.aggregations.bucket.global.Global; import org.elasticsearch.search.aggregations.bucket.global.Global;
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
import org.elasticsearch.search.aggregations.metrics.valuecount.ValueCount; import org.elasticsearch.search.aggregations.metrics.valuecount.ValueCount;
import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.elasticsearch.index.query.QueryBuilders.termQuery;
import static org.elasticsearch.search.aggregations.AggregationBuilders.count; import static org.elasticsearch.search.aggregations.AggregationBuilders.count;
import static org.elasticsearch.search.aggregations.AggregationBuilders.filter;
import static org.elasticsearch.search.aggregations.AggregationBuilders.global; import static org.elasticsearch.search.aggregations.AggregationBuilders.global;
import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.METRIC_SCRIPT_ENGINE; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.METRIC_SCRIPT_ENGINE;
import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.SUM_FIELD_PARAMS_SCRIPT; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.SUM_FIELD_PARAMS_SCRIPT;
import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.SUM_VALUES_FIELD_SCRIPT; import static org.elasticsearch.search.aggregations.metrics.MetricAggScriptPlugin.SUM_VALUES_FIELD_SCRIPT;
@ -243,4 +250,33 @@ public class ValueCountIT extends ESIntegTestCase {
assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache() assertThat(client().admin().indices().prepareStats("cache_test_idx").setRequestCache(true).get().getTotal().getRequestCache()
.getMissCount(), equalTo(1L)); .getMissCount(), equalTo(1L));
} }
public void testOrderByEmptyAggregation() throws Exception {
SearchResponse searchResponse = client().prepareSearch("idx").setQuery(matchAllQuery())
.addAggregation(terms("terms").field("value").order(BucketOrder.compound(BucketOrder.aggregation("filter>count", true)))
.subAggregation(filter("filter", termQuery("value", 100)).subAggregation(count("count").field("value"))))
.get();
assertHitCount(searchResponse, 10);
Terms terms = searchResponse.getAggregations().get("terms");
assertThat(terms, notNullValue());
List<? extends Terms.Bucket> buckets = terms.getBuckets();
assertThat(buckets, notNullValue());
assertThat(buckets.size(), equalTo(10));
for (int i = 0; i < 10; i++) {
Terms.Bucket bucket = buckets.get(i);
assertThat(bucket, notNullValue());
assertThat(bucket.getKeyAsNumber(), equalTo((long) i + 1));
assertThat(bucket.getDocCount(), equalTo(1L));
Filter filter = bucket.getAggregations().get("filter");
assertThat(filter, notNullValue());
assertThat(filter.getDocCount(), equalTo(0L));
ValueCount count = filter.getAggregations().get("count");
assertThat(count, notNullValue());
assertThat(count.value(), equalTo(0.0));
}
}
} }