From f522172ae3484f182391a6b43dfab6550ba4b81a Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 21 Oct 2014 16:29:20 +0200 Subject: [PATCH] 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 --- .../children/ParentToChildrenAggregator.java | 8 +++- .../aggregations/bucket/ChildrenTests.java | 43 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) 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;