Skip to content

Commit 2527bf0

Browse files
martijnvgmartijnvg
committed
inner hits: Do not allow inner hits to be specified on a nested that has a object field
as parent field and that parent field is defined as an array field in the _source of the document inner hits are being computed for. Closes #25315
1 parent b11101a commit 2527bf0

File tree

6 files changed

+114
-34
lines changed

6 files changed

+114
-34
lines changed

‎core/src/main/java/org/elasticsearch/index/cache/bitset/BitsetFilterCache.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ public IndexWarmer.TerminationHandle warmReader(final IndexShard indexShard, fin
239239
hasNested = true;
240240
for (ObjectMapper objectMapper : docMapper.objectMappers().values()) {
241241
if (objectMapper.nested().isNested()) {
242-
ObjectMapper parentObjectMapper = docMapper.findParentObjectMapper(objectMapper);
242+
ObjectMapper parentObjectMapper = objectMapper.getParentObjectMapper(mapperService);
243243
if (parentObjectMapper != null && parentObjectMapper.nested().isNested()) {
244244
warmUp.add(parentObjectMapper.nestedTypeFilter());
245245
}

‎core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -296,21 +296,6 @@ public ObjectMapper findNestedObjectMapper(int nestedDocId, SearchContext sc, Le
296296
return nestedObjectMapper;
297297
}
298298

299-
/**
300-
* Returns the parent {@link ObjectMapper} instance of the specified object mapper or <code>null</code> if there
301-
* isn't any.
302-
*/
303-
// TODO: We should add: ObjectMapper#getParentObjectMapper()
304-
public ObjectMapper findParentObjectMapper(ObjectMapper objectMapper) {
305-
int indexOfLastDot = objectMapper.fullPath().lastIndexOf('.');
306-
if (indexOfLastDot != -1) {
307-
String parentNestObjectPath = objectMapper.fullPath().substring(0, indexOfLastDot);
308-
return objectMappers().get(parentNestObjectPath);
309-
} else {
310-
return null;
311-
}
312-
}
313-
314299
public boolean isParent(String type) {
315300
return mapperService.getParentTypes().contains(type);
316301
}

‎core/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,35 @@ public final Dynamic dynamic() {
410410
return dynamic;
411411
}
412412

413+
/**
414+
* Returns the parent {@link ObjectMapper} instance of the specified object mapper or <code>null</code> if there
415+
* isn't any.
416+
*/
417+
public ObjectMapper getParentObjectMapper(MapperService mapperService) {
418+
int indexOfLastDot = fullPath().lastIndexOf('.');
419+
if (indexOfLastDot != -1) {
420+
String parentNestObjectPath = fullPath().substring(0, indexOfLastDot);
421+
return mapperService.getObjectMapper(parentNestObjectPath);
422+
} else {
423+
return null;
424+
}
425+
}
426+
427+
/**
428+
* Returns whether all parent objects fields are nested too.
429+
*/
430+
public boolean parentObjectMapperAreNested(MapperService mapperService) {
431+
for (ObjectMapper parent = getParentObjectMapper(mapperService);
432+
parent != null;
433+
parent = parent.getParentObjectMapper(mapperService)) {
434+
435+
if (parent.nested().isNested() == false) {
436+
return false;
437+
}
438+
}
439+
return true;
440+
}
441+
413442
@Override
414443
public ObjectMapper merge(Mapper mergeWith, boolean updateAllTypes) {
415444
if (!(mergeWith instanceof ObjectMapper)) {

‎core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.elasticsearch.index.fieldvisitor.FieldsVisitor;
4141
import org.elasticsearch.index.mapper.DocumentMapper;
4242
import org.elasticsearch.index.mapper.MappedFieldType;
43+
import org.elasticsearch.index.mapper.MapperService;
4344
import org.elasticsearch.index.mapper.ObjectMapper;
4445
import org.elasticsearch.index.mapper.SourceFieldMapper;
4546
import org.elasticsearch.index.mapper.Uid;
@@ -246,7 +247,7 @@ private SearchHit createNestedSearchHit(SearchContext context, int nestedTopDocI
246247
ObjectMapper nestedObjectMapper = documentMapper.findNestedObjectMapper(nestedSubDocId, context, subReaderContext);
247248
assert nestedObjectMapper != null;
248249
SearchHit.NestedIdentity nestedIdentity =
249-
getInternalNestedIdentity(context, nestedSubDocId, subReaderContext, documentMapper, nestedObjectMapper);
250+
getInternalNestedIdentity(context, nestedSubDocId, subReaderContext, context.mapperService(), nestedObjectMapper);
250251

251252
if (source != null) {
252253
Tuple<XContentType, Map<String, Object>> tuple = XContentHelper.convertToMap(source, true);
@@ -262,18 +263,28 @@ private SearchHit createNestedSearchHit(SearchContext context, int nestedTopDocI
262263
String nestedPath = nested.getField().string();
263264
current.put(nestedPath, new HashMap<>());
264265
Object extractedValue = XContentMapValues.extractValue(nestedPath, sourceAsMap);
265-
List<Map<String, Object>> nestedParsedSource;
266+
List<?> nestedParsedSource;
266267
if (extractedValue instanceof List) {
267268
// nested field has an array value in the _source
268-
nestedParsedSource = (List<Map<String, Object>>) extractedValue;
269+
nestedParsedSource = (List<?>) extractedValue;
269270
} else if (extractedValue instanceof Map) {
270271
// nested field has an object value in the _source. This just means the nested field has just one inner object,
271272
// which is valid, but uncommon.
272-
nestedParsedSource = Collections.singletonList((Map<String, Object>) extractedValue);
273+
nestedParsedSource = Collections.singletonList(extractedValue);
273274
} else {
274275
throw new IllegalStateException("extracted source isn't an object or an array");
275276
}
276-
sourceAsMap = nestedParsedSource.get(nested.getOffset());
277+
if ((nestedParsedSource.get(0) instanceof Map) == false &&
278+
nestedObjectMapper.parentObjectMapperAreNested(context.mapperService()) == false) {
279+
// When one of the parent objects are not nested then XContentMapValues.extractValue(...) extracts the values
280+
// from two or more layers resulting in a list of list being returned. This is because nestedPath
281+
// encapsulates two or more object layers in the _source.
282+
//
283+
// This is why only the first element of nestedParsedSource needs to be checked.
284+
throw new IllegalArgumentException("Cannot execute inner hits. One or more parent object fields of nested field [" +
285+
nestedObjectMapper.name() + "] are not nested. All parent fields need to be nested fields too");
286+
}
287+
sourceAsMap = (Map<String, Object>) nestedParsedSource.get(nested.getOffset());
277288
if (nested.getChild() == null) {
278289
current.put(nestedPath, sourceAsMap);
279290
} else {
@@ -312,7 +323,8 @@ private Map<String, DocumentField> getSearchFields(SearchContext context, int ne
312323
}
313324

314325
private SearchHit.NestedIdentity getInternalNestedIdentity(SearchContext context, int nestedSubDocId,
315-
LeafReaderContext subReaderContext, DocumentMapper documentMapper,
326+
LeafReaderContext subReaderContext,
327+
MapperService mapperService,
316328
ObjectMapper nestedObjectMapper) throws IOException {
317329
int currentParent = nestedSubDocId;
318330
ObjectMapper nestedParentObjectMapper;
@@ -321,7 +333,7 @@ private SearchHit.NestedIdentity getInternalNestedIdentity(SearchContext context
321333
SearchHit.NestedIdentity nestedIdentity = null;
322334
do {
323335
Query parentFilter;
324-
nestedParentObjectMapper = documentMapper.findParentObjectMapper(current);
336+
nestedParentObjectMapper = current.getParentObjectMapper(mapperService);
325337
if (nestedParentObjectMapper != null) {
326338
if (nestedParentObjectMapper.nested().isNested() == false) {
327339
current = nestedParentObjectMapper;

‎core/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.Collections;
3737
import java.util.function.Function;
3838

39+
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
3940
import static org.hamcrest.Matchers.containsString;
4041
import static org.hamcrest.Matchers.equalTo;
4142
import static org.hamcrest.Matchers.nullValue;
@@ -428,4 +429,35 @@ public void testLimitOfNestedFieldsWithMultiTypePerIndex() throws Exception {
428429
createIndex("test5", Settings.builder().put(MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING.getKey(), 0).build())
429430
.mapperService().merge("type", new CompressedXContent(mapping.apply("type")), MergeReason.MAPPING_RECOVERY, false);
430431
}
432+
433+
public void testParentObjectMapperAreNested() throws Exception {
434+
MapperService mapperService = createIndex("index1", Settings.EMPTY, "doc", jsonBuilder().startObject()
435+
.startObject("properties")
436+
.startObject("comments")
437+
.field("type", "nested")
438+
.startObject("properties")
439+
.startObject("messages")
440+
.field("type", "nested").endObject()
441+
.endObject()
442+
.endObject()
443+
.endObject()
444+
.endObject()).mapperService();
445+
ObjectMapper objectMapper = mapperService.getObjectMapper("comments.messages");
446+
assertTrue(objectMapper.parentObjectMapperAreNested(mapperService));
447+
448+
mapperService = createIndex("index2", Settings.EMPTY, "doc", jsonBuilder().startObject()
449+
.startObject("properties")
450+
.startObject("comments")
451+
.field("type", "object")
452+
.startObject("properties")
453+
.startObject("messages")
454+
.field("type", "nested").endObject()
455+
.endObject()
456+
.endObject()
457+
.endObject()
458+
.endObject()).mapperService();
459+
objectMapper = mapperService.getObjectMapper("comments.messages");
460+
assertFalse(objectMapper.parentObjectMapperAreNested(mapperService));
461+
}
462+
431463
}

‎core/src/test/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java‎

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -402,32 +402,54 @@ public void testInnerHitsWithObjectFieldThatHasANestedField() throws Exception {
402402
List<IndexRequestBuilder> requests = new ArrayList<>();
403403
requests.add(client().prepareIndex("articles", "article", "1").setSource(jsonBuilder().startObject()
404404
.field("title", "quick brown fox")
405-
.startObject("comments")
406-
.startArray("messages")
407-
.startObject().field("message", "fox eat quick").endObject()
408-
.startObject().field("message", "bear eat quick").endObject()
405+
.startArray("comments")
406+
.startObject()
407+
.startArray("messages")
408+
.startObject().field("message", "fox eat quick").endObject()
409+
.startObject().field("message", "bear eat quick").endObject()
410+
.endArray()
411+
.endObject()
412+
.startObject()
413+
.startArray("messages")
414+
.startObject().field("message", "no fox").endObject()
415+
.endArray()
416+
.endObject()
409417
.endArray()
410-
.endObject()
411418
.endObject()));
412419
indexRandom(true, requests);
413420

414-
SearchResponse response = client().prepareSearch("articles")
421+
SearchResponse response = client().prepareSearch("articles").setQuery(nestedQuery("comments.messages",
422+
matchQuery("comments.messages.message", "fox"), ScoreMode.Avg).innerHit(new InnerHitBuilder())).get();
423+
assertEquals("Cannot execute inner hits. One or more parent object fields of nested field [comments.messages] are " +
424+
"not nested. All parent fields need to be nested fields too", response.getShardFailures()[0].getCause().getMessage());
425+
426+
response = client().prepareSearch("articles").setQuery(nestedQuery("comments.messages",
427+
matchQuery("comments.messages.message", "fox"), ScoreMode.Avg).innerHit(new InnerHitBuilder()
428+
.setFetchSourceContext(new FetchSourceContext(true)))).get();
429+
assertEquals("Cannot execute inner hits. One or more parent object fields of nested field [comments.messages] are " +
430+
"not nested. All parent fields need to be nested fields too", response.getShardFailures()[0].getCause().getMessage());
431+
432+
response = client().prepareSearch("articles")
415433
.setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "fox"), ScoreMode.Avg)
416-
.innerHit(new InnerHitBuilder())).get();
434+
.innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(false)))).get();
417435
assertNoFailures(response);
418436
assertHitCount(response, 1);
419437
SearchHit hit = response.getHits().getAt(0);
420438
assertThat(hit.getId(), equalTo("1"));
421439
SearchHits messages = hit.getInnerHits().get("comments.messages");
422-
assertThat(messages.getTotalHits(), equalTo(1L));
440+
assertThat(messages.getTotalHits(), equalTo(2L));
423441
assertThat(messages.getAt(0).getId(), equalTo("1"));
424442
assertThat(messages.getAt(0).getNestedIdentity().getField().string(), equalTo("comments.messages"));
425-
assertThat(messages.getAt(0).getNestedIdentity().getOffset(), equalTo(0));
443+
assertThat(messages.getAt(0).getNestedIdentity().getOffset(), equalTo(2));
426444
assertThat(messages.getAt(0).getNestedIdentity().getChild(), nullValue());
445+
assertThat(messages.getAt(1).getId(), equalTo("1"));
446+
assertThat(messages.getAt(1).getNestedIdentity().getField().string(), equalTo("comments.messages"));
447+
assertThat(messages.getAt(1).getNestedIdentity().getOffset(), equalTo(0));
448+
assertThat(messages.getAt(1).getNestedIdentity().getChild(), nullValue());
427449

428450
response = client().prepareSearch("articles")
429451
.setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "bear"), ScoreMode.Avg)
430-
.innerHit(new InnerHitBuilder())).get();
452+
.innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(false)))).get();
431453
assertNoFailures(response);
432454
assertHitCount(response, 1);
433455
hit = response.getHits().getAt(0);
@@ -448,7 +470,7 @@ public void testInnerHitsWithObjectFieldThatHasANestedField() throws Exception {
448470
indexRandom(true, requests);
449471
response = client().prepareSearch("articles")
450472
.setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "fox"), ScoreMode.Avg)
451-
.innerHit(new InnerHitBuilder())).get();
473+
.innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(false)))).get();
452474
assertNoFailures(response);
453475
assertHitCount(response, 1);
454476
hit = response.getHits().getAt(0);;

0 commit comments

Comments
 (0)