Aggregations: Fixed bug in top_hits aggregation to not fail with NPE when shard results are empty.

The top_hits aggregation returned an empty InternalTopHits instance with no fields set when there were no result, causing reduce and serialization errors down the road. This is fixed by setting all required fields when a there are no results.

Closes #6346
This commit is contained in:
Martijn van Groningen 2014-05-30 11:09:01 +02:00
parent 74eff87dd6
commit 35755cd8a4
6 changed files with 24 additions and 4 deletions

View File

@ -73,7 +73,7 @@ public class InternalTopHits extends InternalAggregation implements TopHits, ToX
public InternalTopHits(String name, InternalSearchHits searchHits) { public InternalTopHits(String name, InternalSearchHits searchHits) {
this.name = name; this.name = name;
this.searchHits = searchHits; this.searchHits = searchHits;
this.topDocs = new TopDocs(0, Lucene.EMPTY_SCORE_DOCS, 0); this.topDocs = Lucene.EMPTY_TOP_DOCS;
} }

View File

@ -23,6 +23,7 @@ import org.apache.lucene.index.AtomicReaderContext;
import org.apache.lucene.search.*; import org.apache.lucene.search.*;
import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.lease.Releasables;
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.common.lucene.ScorerAware; import org.elasticsearch.common.lucene.ScorerAware;
import org.elasticsearch.common.util.LongObjectPagedHashMap; import org.elasticsearch.common.util.LongObjectPagedHashMap;
import org.elasticsearch.search.aggregations.*; import org.elasticsearch.search.aggregations.*;
@ -31,6 +32,7 @@ import org.elasticsearch.search.aggregations.support.AggregationContext;
import org.elasticsearch.search.fetch.FetchPhase; import org.elasticsearch.search.fetch.FetchPhase;
import org.elasticsearch.search.fetch.FetchSearchResult; import org.elasticsearch.search.fetch.FetchSearchResult;
import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.InternalSearchHit;
import org.elasticsearch.search.internal.InternalSearchHits;
import java.io.IOException; import java.io.IOException;
@ -94,7 +96,7 @@ public class TopHitsAggregator extends BucketsAggregator implements ScorerAware
@Override @Override
public InternalAggregation buildEmptyAggregation() { public InternalAggregation buildEmptyAggregation() {
return new InternalTopHits(); return new InternalTopHits(name, topHitsContext.size(), topHitsContext.sort(), Lucene.EMPTY_TOP_DOCS, InternalSearchHits.empty());
} }
@Override @Override

View File

@ -368,7 +368,7 @@ public class TopHitsBuilder extends AbstractAggregationBuilder {
@Override @Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name).field(type); builder.startObject(name).field(type);
sourceBuilder.toXContent(builder, params); sourceBuilder().toXContent(builder, params);
return builder.endObject(); return builder.endObject();
} }

View File

@ -90,6 +90,10 @@ public class InternalSearchHits implements SearchHits {
return cache.get().reset(); return cache.get().reset();
} }
public static InternalSearchHits empty() {
// We shouldn't use static final instance, since that could directly be returned by native transport clients
return new InternalSearchHits(EMPTY, 0, 0);
}
public static final InternalSearchHit[] EMPTY = new InternalSearchHit[0]; public static final InternalSearchHit[] EMPTY = new InternalSearchHit[0];

View File

@ -41,7 +41,7 @@ import static org.elasticsearch.search.internal.InternalSearchHits.readSearchHit
public class InternalSearchResponse implements Streamable, ToXContent { public class InternalSearchResponse implements Streamable, ToXContent {
public static InternalSearchResponse empty() { public static InternalSearchResponse empty() {
return new InternalSearchResponse(new InternalSearchHits(new InternalSearchHit[0], 0, 0), null, null, null, false); return new InternalSearchResponse(InternalSearchHits.empty(), null, null, null, false);
} }
private InternalSearchHits hits; private InternalSearchHits hits;

View File

@ -64,6 +64,7 @@ public class TopHitsTests extends ElasticsearchIntegrationTest {
@Override @Override
public void setupSuiteScopeCluster() throws Exception { public void setupSuiteScopeCluster() throws Exception {
createIndex("idx"); createIndex("idx");
createIndex("empty");
List<IndexRequestBuilder> builders = new ArrayList<>(); List<IndexRequestBuilder> builders = new ArrayList<>();
for (int i = 0; i < 50; i++) { for (int i = 0; i < 50; i++) {
builders.add(client().prepareIndex("idx", "type", Integer.toString(i)).setSource(jsonBuilder() builders.add(client().prepareIndex("idx", "type", Integer.toString(i)).setSource(jsonBuilder()
@ -363,4 +364,17 @@ public class TopHitsTests extends ElasticsearchIntegrationTest {
} }
} }
@Test
public void testEmptyIndex() throws Exception {
SearchResponse response = client().prepareSearch("empty").setTypes("type")
.addAggregation(topHits("hits"))
.get();
assertSearchResponse(response);
TopHits hits = response.getAggregations().get("hits");
assertThat(hits, notNullValue());
assertThat(hits.getName(), equalTo("hits"));
assertThat(hits.getHits().totalHits(), equalTo(0l));
}
} }