aggs: Fix 2 bug in `children` agg

1) multiple nested children aggs result in a NPE
2) fixed a counting bug where the same readers where post collected twice

Closes #10158
This commit is contained in:
Martijn van Groningen 2015-03-25 14:55:33 +01:00
parent 3df3259a74
commit 16d8f3ce81
2 changed files with 53 additions and 11 deletions

View File

@ -30,11 +30,7 @@ import org.elasticsearch.common.lucene.docset.DocIdSets;
import org.elasticsearch.common.util.LongArray;
import org.elasticsearch.common.util.LongObjectPagedHashMap;
import org.elasticsearch.index.search.child.ConstantScorer;
import org.elasticsearch.search.aggregations.Aggregator;
import org.elasticsearch.search.aggregations.AggregatorFactories;
import org.elasticsearch.search.aggregations.InternalAggregation;
import org.elasticsearch.search.aggregations.LeafBucketCollector;
import org.elasticsearch.search.aggregations.NonCollectingAggregator;
import org.elasticsearch.search.aggregations.*;
import org.elasticsearch.search.aggregations.bucket.SingleBucketAggregator;
import org.elasticsearch.search.aggregations.support.AggregationContext;
import org.elasticsearch.search.aggregations.support.ValuesSource;
@ -42,10 +38,7 @@ import org.elasticsearch.search.aggregations.support.ValuesSourceAggregatorFacto
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.*;
// The RecordingPerReaderBucketCollector assumes per segment recording which isn't the case for this
// aggregation, for this reason that collector can't be used
@ -66,7 +59,7 @@ public class ParentToChildrenAggregator extends SingleBucketAggregator {
private final LongObjectPagedHashMap<long[]> parentOrdToOtherBuckets;
private boolean multipleBucketsPerParentOrd = false;
private List<LeafReaderContext> replay = new ArrayList<>();
private Set<LeafReaderContext> replay = new LinkedHashSet<>();
public ParentToChildrenAggregator(String name, AggregatorFactories factories, AggregationContext aggregationContext,
Aggregator parent, String parentType, Filter childFilter, Filter parentFilter,
@ -141,7 +134,7 @@ public class ParentToChildrenAggregator extends SingleBucketAggregator {
@Override
protected void doPostCollection() throws IOException {
final List<LeafReaderContext> replay = this.replay;
final Set<LeafReaderContext> replay = this.replay;
this.replay = null;
for (LeafReaderContext ctx : replay) {

View File

@ -21,6 +21,8 @@ package org.elasticsearch.search.aggregations.bucket;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.update.UpdateResponse;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.aggregations.bucket.children.Children;
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
@ -335,6 +337,53 @@ public class ChildrenTests extends ElasticsearchIntegrationTest {
assertThat(termsAgg.getBucketByKey("44").getDocCount(), equalTo(1l));
}
@Test
public void testHierarchicalChildrenAggs() {
String indexName = "geo";
String grandParentType = "continent";
String parentType = "country";
String childType = "city";
assertAcked(
prepareCreate(indexName)
.setSettings(ImmutableSettings.builder()
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0)
)
.addMapping(grandParentType)
.addMapping(parentType, "_parent", "type=" + grandParentType)
.addMapping(childType, "_parent", "type=" + parentType)
);
client().prepareIndex(indexName, grandParentType, "1").setSource("name", "europe").get();
client().prepareIndex(indexName, parentType, "2").setParent("1").setSource("name", "belgium").get();
client().prepareIndex(indexName, childType, "3").setParent("2").setRouting("1").setSource("name", "brussels").get();
refresh();
SearchResponse response = client().prepareSearch(indexName)
.setQuery(matchQuery("name", "europe"))
.addAggregation(
children(parentType).childType(parentType).subAggregation(
children(childType).childType(childType).subAggregation(
terms("name").field("name")
)
)
)
.get();
assertNoFailures(response);
assertHitCount(response, 1);
Children children = response.getAggregations().get(parentType);
assertThat(children.getName(), equalTo(parentType));
assertThat(children.getDocCount(), equalTo(1l));
children = children.getAggregations().get(childType);
assertThat(children.getName(), equalTo(childType));
assertThat(children.getDocCount(), equalTo(1l));
Terms terms = children.getAggregations().get("name");
assertThat(terms.getBuckets().size(), equalTo(1));
assertThat(terms.getBuckets().get(0).getKey().toString(), equalTo("brussels"));
assertThat(terms.getBuckets().get(0).getDocCount(), equalTo(1l));
}
private static final class Control {
final String category;