From 16d8f3ce813192181299fe193c27f2de8459d21e Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 25 Mar 2015 14:55:33 +0100 Subject: [PATCH] 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 --- .../children/ParentToChildrenAggregator.java | 15 ++---- .../aggregations/bucket/ChildrenTests.java | 49 +++++++++++++++++++ 2 files changed, 53 insertions(+), 11 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 b4769f05baf..bc7e663779f 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 @@ -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 parentOrdToOtherBuckets; private boolean multipleBucketsPerParentOrd = false; - private List replay = new ArrayList<>(); + private Set 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 replay = this.replay; + final Set replay = this.replay; this.replay = null; for (LeafReaderContext ctx : replay) { 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 f4dca96f239..ce2e12e9376 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/ChildrenTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/ChildrenTests.java @@ -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;