Aggregations: the children agg didn't take deleted document into account.

The live docs that is passed down was ignored by the filter impl. Now the children filter gets wrapped with ApplyAcceptedDocsFilter, so live docs are actually applied.

Closes #8180

Author: martijnvg

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);

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<IndexRequestBuilder> 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;