Skip to content

Commit 66f0fa8

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 72c334f commit 66f0fa8

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
@@ -235,7 +235,7 @@ public IndexWarmer.TerminationHandle warmReader(final IndexShard indexShard, fin
235235
hasNested = true;
236236
for (ObjectMapper objectMapper : docMapper.objectMappers().values()) {
237237
if (objectMapper.nested().isNested()) {
238-
ObjectMapper parentObjectMapper = docMapper.findParentObjectMapper(objectMapper);
238+
ObjectMapper parentObjectMapper = objectMapper.getParentObjectMapper(mapperService);
239239
if (parentObjectMapper != null && parentObjectMapper.nested().isNested()) {
240240
warmUp.add(parentObjectMapper.nestedTypeFilter());
241241
}

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

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

315-
/**
316-
* Returns the parent {@link ObjectMapper} instance of the specified object mapper or <code>null</code> if there
317-
* isn't any.
318-
*/
319-
// TODO: We should add: ObjectMapper#getParentObjectMapper()
320-
public ObjectMapper findParentObjectMapper(ObjectMapper objectMapper) {
321-
int indexOfLastDot = objectMapper.fullPath().lastIndexOf('.');
322-
if (indexOfLastDot != -1) {
323-
String parentNestObjectPath = objectMapper.fullPath().substring(0, indexOfLastDot);
324-
return objectMappers().get(parentNestObjectPath);
325-
} else {
326-
return null;
327-
}
328-
}
329-
330315
public boolean isParent(String type) {
331316
return mapperService.getParentTypes().contains(type);
332317
}

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

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

412+
/**
413+
* Returns the parent {@link ObjectMapper} instance of the specified object mapper or <code>null</code> if there
414+
* isn't any.
415+
*/
416+
public ObjectMapper getParentObjectMapper(MapperService mapperService) {
417+
int indexOfLastDot = fullPath().lastIndexOf('.');
418+
if (indexOfLastDot != -1) {
419+
String parentNestObjectPath = fullPath().substring(0, indexOfLastDot);
420+
return mapperService.getObjectMapper(parentNestObjectPath);
421+
} else {
422+
return null;
423+
}
424+
}
425+
426+
/**
427+
* Returns whether all parent objects fields are nested too.
428+
*/
429+
public boolean parentObjectMapperAreNested(MapperService mapperService) {
430+
for (ObjectMapper parent = getParentObjectMapper(mapperService);
431+
parent != null;
432+
parent = parent.getParentObjectMapper(mapperService)) {
433+
434+
if (parent.nested().isNested() == false) {
435+
return false;
436+
}
437+
}
438+
return true;
439+
}
440+
412441
@Override
413442
public ObjectMapper merge(Mapper mergeWith, boolean updateAllTypes) {
414443
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
@@ -39,6 +39,7 @@
3939
import org.elasticsearch.index.fieldvisitor.FieldsVisitor;
4040
import org.elasticsearch.index.mapper.DocumentMapper;
4141
import org.elasticsearch.index.mapper.MappedFieldType;
42+
import org.elasticsearch.index.mapper.MapperService;
4243
import org.elasticsearch.index.mapper.ObjectMapper;
4344
import org.elasticsearch.index.mapper.SourceFieldMapper;
4445
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, SearchHitField> getSearchFields(SearchContext context, int n
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
@@ -30,6 +30,7 @@
3030
import java.io.UncheckedIOException;
3131
import java.util.function.Function;
3232

33+
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
3334
import static org.hamcrest.Matchers.containsString;
3435
import static org.hamcrest.Matchers.equalTo;
3536
import static org.hamcrest.Matchers.nullValue;
@@ -388,4 +389,35 @@ public void testLimitOfNestedFieldsPerIndex() throws Exception {
388389
createIndex("test5", Settings.builder().put(MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING.getKey(), 0).build())
389390
.mapperService().merge("type", new CompressedXContent(mapping.apply("type")), MergeReason.MAPPING_RECOVERY, false);
390391
}
392+
393+
public void testParentObjectMapperAreNested() throws Exception {
394+
MapperService mapperService = createIndex("index1", Settings.EMPTY, "doc", jsonBuilder().startObject()
395+
.startObject("properties")
396+
.startObject("comments")
397+
.field("type", "nested")
398+
.startObject("properties")
399+
.startObject("messages")
400+
.field("type", "nested").endObject()
401+
.endObject()
402+
.endObject()
403+
.endObject()
404+
.endObject()).mapperService();
405+
ObjectMapper objectMapper = mapperService.getObjectMapper("comments.messages");
406+
assertTrue(objectMapper.parentObjectMapperAreNested(mapperService));
407+
408+
mapperService = createIndex("index2", Settings.EMPTY, "doc", jsonBuilder().startObject()
409+
.startObject("properties")
410+
.startObject("comments")
411+
.field("type", "object")
412+
.startObject("properties")
413+
.startObject("messages")
414+
.field("type", "nested").endObject()
415+
.endObject()
416+
.endObject()
417+
.endObject()
418+
.endObject()).mapperService();
419+
objectMapper = mapperService.getObjectMapper("comments.messages");
420+
assertFalse(objectMapper.parentObjectMapperAreNested(mapperService));
421+
}
422+
391423
}

‎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
@@ -407,32 +407,54 @@ public void testInnerHitsWithObjectFieldThatHasANestedField() throws Exception {
407407
List<IndexRequestBuilder> requests = new ArrayList<>();
408408
requests.add(client().prepareIndex("articles", "article", "1").setSource(jsonBuilder().startObject()
409409
.field("title", "quick brown fox")
410-
.startObject("comments")
411-
.startArray("messages")
412-
.startObject().field("message", "fox eat quick").endObject()
413-
.startObject().field("message", "bear eat quick").endObject()
410+
.startArray("comments")
411+
.startObject()
412+
.startArray("messages")
413+
.startObject().field("message", "fox eat quick").endObject()
414+
.startObject().field("message", "bear eat quick").endObject()
415+
.endArray()
416+
.endObject()
417+
.startObject()
418+
.startArray("messages")
419+
.startObject().field("message", "no fox").endObject()
420+
.endArray()
421+
.endObject()
414422
.endArray()
415-
.endObject()
416423
.endObject()));
417424
indexRandom(true, requests);
418425

419-
SearchResponse response = client().prepareSearch("articles")
426+
SearchResponse response = client().prepareSearch("articles").setQuery(nestedQuery("comments.messages",
427+
matchQuery("comments.messages.message", "fox"), ScoreMode.Avg).innerHit(new InnerHitBuilder())).get();
428+
assertEquals("Cannot execute inner hits. One or more parent object fields of nested field [comments.messages] are " +
429+
"not nested. All parent fields need to be nested fields too", response.getShardFailures()[0].getCause().getMessage());
430+
431+
response = client().prepareSearch("articles").setQuery(nestedQuery("comments.messages",
432+
matchQuery("comments.messages.message", "fox"), ScoreMode.Avg).innerHit(new InnerHitBuilder()
433+
.setFetchSourceContext(new FetchSourceContext(true)))).get();
434+
assertEquals("Cannot execute inner hits. One or more parent object fields of nested field [comments.messages] are " +
435+
"not nested. All parent fields need to be nested fields too", response.getShardFailures()[0].getCause().getMessage());
436+
437+
response = client().prepareSearch("articles")
420438
.setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "fox"), ScoreMode.Avg)
421-
.innerHit(new InnerHitBuilder())).get();
439+
.innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(false)))).get();
422440
assertNoFailures(response);
423441
assertHitCount(response, 1);
424442
SearchHit hit = response.getHits().getAt(0);
425443
assertThat(hit.id(), equalTo("1"));
426444
SearchHits messages = hit.getInnerHits().get("comments.messages");
427-
assertThat(messages.getTotalHits(), equalTo(1L));
445+
assertThat(messages.getTotalHits(), equalTo(2L));
428446
assertThat(messages.getAt(0).id(), equalTo("1"));
429447
assertThat(messages.getAt(0).getNestedIdentity().getField().string(), equalTo("comments.messages"));
430-
assertThat(messages.getAt(0).getNestedIdentity().getOffset(), equalTo(0));
448+
assertThat(messages.getAt(0).getNestedIdentity().getOffset(), equalTo(2));
431449
assertThat(messages.getAt(0).getNestedIdentity().getChild(), nullValue());
450+
assertThat(messages.getAt(1).getId(), equalTo("1"));
451+
assertThat(messages.getAt(1).getNestedIdentity().getField().string(), equalTo("comments.messages"));
452+
assertThat(messages.getAt(1).getNestedIdentity().getOffset(), equalTo(0));
453+
assertThat(messages.getAt(1).getNestedIdentity().getChild(), nullValue());
432454

433455
response = client().prepareSearch("articles")
434456
.setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "bear"), ScoreMode.Avg)
435-
.innerHit(new InnerHitBuilder())).get();
457+
.innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(false)))).get();
436458
assertNoFailures(response);
437459
assertHitCount(response, 1);
438460
hit = response.getHits().getAt(0);
@@ -453,7 +475,7 @@ public void testInnerHitsWithObjectFieldThatHasANestedField() throws Exception {
453475
indexRandom(true, requests);
454476
response = client().prepareSearch("articles")
455477
.setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "fox"), ScoreMode.Avg)
456-
.innerHit(new InnerHitBuilder())).get();
478+
.innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(false)))).get();
457479
assertNoFailures(response);
458480
assertHitCount(response, 1);
459481
hit = response.getHits().getAt(0);;

0 commit comments

Comments
 (0)