Skip to content

Commit

Permalink
inner_hits: Return an empty _source for nested inner hit when filteri…
Browse files Browse the repository at this point in the history
…ng on a field that doesn't exist.

Before this change the search request would fail with an error indicating that it couldn't detect xcontent type based on the string: `null`
  • Loading branch information
martijnvg committed Nov 27, 2017
1 parent 4ab638b commit 3f98b85
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,16 @@
package org.elasticsearch.search.fetch.subphase;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.fetch.FetchSubPhase;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.search.lookup.SourceLookup;

import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.Map;

import static org.elasticsearch.common.xcontent.XContentFactory.contentBuilder;

public final class FetchSourceSubPhase implements FetchSubPhase {

@Override
Expand Down Expand Up @@ -65,7 +60,17 @@ public void hitExecute(SearchContext context, HitContext hitContext) {
final int initialCapacity = nestedHit ? 1024 : Math.min(1024, source.internalSourceRef().length());
BytesStreamOutput streamOutput = new BytesStreamOutput(initialCapacity);
XContentBuilder builder = new XContentBuilder(source.sourceContentType().xContent(), streamOutput);
builder.value(value);
if (value != null) {
builder.value(value);
} else {
// This happens if the source filtering could not find the specified in the _source.
// Just doing `builder.value(null)` is valid, but the xcontent validation can't detect what format
// it is. In certain cases, for example response serialization we fail if no xcontent type can't be
// detected. So instead we just return an empty top level object. Also this is in inline with what was
// being return in this situation in 5.x and earlier.
builder.startObject();
builder.endObject();
}
hitContext.hit().sourceRef(builder.bytes());
} catch (IOException e) {
throw new ElasticsearchException("Error filtering source", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@

import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.common.xcontent.support.XContentMapValues.extractValue;
import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchQuery;
Expand Down Expand Up @@ -635,6 +634,18 @@ public void testNestedSource() throws Exception {
assertThat(response.getHits().getAt(0).getInnerHits().get("comments").getAt(0).getSourceAsMap().size(), equalTo(2));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments").getAt(1).getSourceAsMap().get("message"),
equalTo("fox ate rabbit x y z"));

// Source filter on a field that does not exist inside the nested document and just check that we do not fail and
// return an empty _source:
response = client().prepareSearch()
.setQuery(nestedQuery("comments", matchQuery("comments.message", "away"), ScoreMode.None)
.innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(true,
new String[]{"comments.missing_field"}, null))))
.get();
assertNoFailures(response);
assertHitCount(response, 1);
assertThat(response.getHits().getAt(0).getInnerHits().get("comments").getTotalHits(), equalTo(1L));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments").getAt(0).getSourceAsMap().size(), equalTo(0));
}

public void testInnerHitsWithIgnoreUnmapped() throws Exception {
Expand Down

0 comments on commit 3f98b85

Please sign in to comment.