diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/children/ParentToChildrenAggregator.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/children/ParentToChildrenAggregator.java index c055d0ece0a1d..e29d311057228 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/children/ParentToChildrenAggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/children/ParentToChildrenAggregator.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.lucene.ReaderContextAware; import org.elasticsearch.common.lucene.docset.DocIdSets; +import org.elasticsearch.common.lucene.search.ApplyAcceptedDocsFilter; import org.elasticsearch.common.util.LongArray; import org.elasticsearch.common.util.LongObjectPagedHashMap; import org.elasticsearch.index.cache.fixedbitset.FixedBitSetFilter; @@ -52,7 +53,7 @@ public class ParentToChildrenAggregator extends SingleBucketAggregator implements ReaderContextAware { private final String parentType; - private final FixedBitSetFilter childFilter; + private final Filter childFilter; private final FixedBitSetFilter parentFilter; private final ValuesSource.Bytes.WithOrdinals.ParentChild valuesSource; @@ -75,7 +76,10 @@ public ParentToChildrenAggregator(String name, AggregatorFactories factories, Ag ValuesSource.Bytes.WithOrdinals.ParentChild valuesSource, long maxOrd) { super(name, factories, aggregationContext, parent); this.parentType = parentType; - this.childFilter = aggregationContext.searchContext().fixedBitSetFilterCache().getFixedBitSetFilter(childFilter); + // The child filter doesn't rely on random access it just used to iterate over all docs with a specific type, + // so use the filter cache instead. When the filter cache is smarter with what filter impl to pick we can benefit + // from it here + this.childFilter = new ApplyAcceptedDocsFilter(aggregationContext.searchContext().filterCache().cache(childFilter)); this.parentFilter = aggregationContext.searchContext().fixedBitSetFilterCache().getFixedBitSetFilter(parentFilter); this.parentOrdToBuckets = aggregationContext.bigArrays().newLongArray(maxOrd, false); this.parentOrdToBuckets.fill(0, maxOrd, -1); diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/ChildrenTests.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/ChildrenTests.java index 4b9b79b37c25d..bda208cb35572 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/ChildrenTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/ChildrenTests.java @@ -20,9 +20,11 @@ import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.action.update.UpdateResponse; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.aggregations.bucket.children.Children; import org.elasticsearch.search.aggregations.bucket.terms.Terms; +import org.elasticsearch.search.aggregations.metrics.sum.Sum; import org.elasticsearch.search.aggregations.metrics.tophits.TopHits; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.test.ElasticsearchIntegrationTest; @@ -33,8 +35,10 @@ import static org.elasticsearch.index.query.QueryBuilders.matchQuery; import static org.elasticsearch.search.aggregations.AggregationBuilders.*; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; /** @@ -215,6 +219,45 @@ public void testParentWithMultipleBuckets() throws Exception { assertThat(topHits.getHits().getAt(0).getType(), equalTo("comment")); } + @Test + public void testWithDeletes() throws Exception { + String indexName = "xyz"; + assertAcked( + prepareCreate(indexName) + .addMapping("parent") + .addMapping("child", "_parent", "type=parent", "count", "type=long") + ); + + List requests = new ArrayList<>(); + requests.add(client().prepareIndex(indexName, "parent", "1").setSource("{}")); + requests.add(client().prepareIndex(indexName, "child", "0").setParent("1").setSource("count", 1)); + requests.add(client().prepareIndex(indexName, "child", "1").setParent("1").setSource("count", 1)); + requests.add(client().prepareIndex(indexName, "child", "2").setParent("1").setSource("count", 1)); + requests.add(client().prepareIndex(indexName, "child", "3").setParent("1").setSource("count", 1)); + indexRandom(true, requests); + + for (int i = 0; i < 10; i++) { + SearchResponse searchResponse = client().prepareSearch(indexName) + .addAggregation(children("children").childType("child").subAggregation(sum("counts").field("count"))) + .get(); + + assertNoFailures(searchResponse); + Children children = searchResponse.getAggregations().get("children"); + assertThat(children.getDocCount(), equalTo(4l)); + + Sum count = children.getAggregations().get("counts"); + assertThat(count.getValue(), equalTo(4.)); + + String idToUpdate = Integer.toString(randomInt(3)); + UpdateResponse updateResponse = client().prepareUpdate(indexName, "child", idToUpdate) + .setParent("1") + .setDoc("count", 1) + .get(); + assertThat(updateResponse.getVersion(), greaterThan(1l)); + refresh(); + } + } + private static final class Control { final String category;