diff --git a/core/src/main/java/org/elasticsearch/index/cache/bitset/BitsetFilterCache.java b/core/src/main/java/org/elasticsearch/index/cache/bitset/BitsetFilterCache.java index 7d3e75f6f5d12..2de8f01dc7153 100644 --- a/core/src/main/java/org/elasticsearch/index/cache/bitset/BitsetFilterCache.java +++ b/core/src/main/java/org/elasticsearch/index/cache/bitset/BitsetFilterCache.java @@ -239,7 +239,7 @@ public IndexWarmer.TerminationHandle warmReader(final IndexShard indexShard, fin hasNested = true; for (ObjectMapper objectMapper : docMapper.objectMappers().values()) { if (objectMapper.nested().isNested()) { - ObjectMapper parentObjectMapper = docMapper.findParentObjectMapper(objectMapper); + ObjectMapper parentObjectMapper = objectMapper.getParentObjectMapper(mapperService); if (parentObjectMapper != null && parentObjectMapper.nested().isNested()) { warmUp.add(parentObjectMapper.nestedTypeFilter()); } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index 1dfb7b19eb259..c4de559d1d956 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -292,21 +292,6 @@ public ObjectMapper findNestedObjectMapper(int nestedDocId, SearchContext sc, Le return nestedObjectMapper; } - /** - * Returns the parent {@link ObjectMapper} instance of the specified object mapper or null if there - * isn't any. - */ - // TODO: We should add: ObjectMapper#getParentObjectMapper() - public ObjectMapper findParentObjectMapper(ObjectMapper objectMapper) { - int indexOfLastDot = objectMapper.fullPath().lastIndexOf('.'); - if (indexOfLastDot != -1) { - String parentNestObjectPath = objectMapper.fullPath().substring(0, indexOfLastDot); - return objectMappers().get(parentNestObjectPath); - } else { - return null; - } - } - public boolean isParent(String type) { return mapperService.getParentTypes().contains(type); } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index fe592835f97a2..d83ce173d6896 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -396,6 +396,35 @@ public final Dynamic dynamic() { return dynamic; } + /** + * Returns the parent {@link ObjectMapper} instance of the specified object mapper or null if there + * isn't any. + */ + public ObjectMapper getParentObjectMapper(MapperService mapperService) { + int indexOfLastDot = fullPath().lastIndexOf('.'); + if (indexOfLastDot != -1) { + String parentNestObjectPath = fullPath().substring(0, indexOfLastDot); + return mapperService.getObjectMapper(parentNestObjectPath); + } else { + return null; + } + } + + /** + * Returns whether all parent objects fields are nested too. + */ + public boolean parentObjectMapperAreNested(MapperService mapperService) { + for (ObjectMapper parent = getParentObjectMapper(mapperService); + parent != null; + parent = parent.getParentObjectMapper(mapperService)) { + + if (parent.nested().isNested() == false) { + return false; + } + } + return true; + } + @Override public ObjectMapper merge(Mapper mergeWith, boolean updateAllTypes) { if (!(mergeWith instanceof ObjectMapper)) { diff --git a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java index 8892a69f2dfc1..f05e38636a0cd 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -40,6 +40,7 @@ import org.elasticsearch.index.fieldvisitor.FieldsVisitor; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.ObjectMapper; import org.elasticsearch.index.mapper.SourceFieldMapper; import org.elasticsearch.index.mapper.Uid; @@ -246,7 +247,7 @@ private SearchHit createNestedSearchHit(SearchContext context, int nestedTopDocI ObjectMapper nestedObjectMapper = documentMapper.findNestedObjectMapper(nestedSubDocId, context, subReaderContext); assert nestedObjectMapper != null; SearchHit.NestedIdentity nestedIdentity = - getInternalNestedIdentity(context, nestedSubDocId, subReaderContext, documentMapper, nestedObjectMapper); + getInternalNestedIdentity(context, nestedSubDocId, subReaderContext, context.mapperService(), nestedObjectMapper); if (source != null) { Tuple> tuple = XContentHelper.convertToMap(source, true); @@ -262,18 +263,28 @@ private SearchHit createNestedSearchHit(SearchContext context, int nestedTopDocI String nestedPath = nested.getField().string(); current.put(nestedPath, new HashMap<>()); Object extractedValue = XContentMapValues.extractValue(nestedPath, sourceAsMap); - List> nestedParsedSource; + List nestedParsedSource; if (extractedValue instanceof List) { // nested field has an array value in the _source - nestedParsedSource = (List>) extractedValue; + nestedParsedSource = (List) extractedValue; } else if (extractedValue instanceof Map) { // nested field has an object value in the _source. This just means the nested field has just one inner object, // which is valid, but uncommon. - nestedParsedSource = Collections.singletonList((Map) extractedValue); + nestedParsedSource = Collections.singletonList(extractedValue); } else { throw new IllegalStateException("extracted source isn't an object or an array"); } - sourceAsMap = nestedParsedSource.get(nested.getOffset()); + if ((nestedParsedSource.get(0) instanceof Map) == false && + nestedObjectMapper.parentObjectMapperAreNested(context.mapperService()) == false) { + // When one of the parent objects are not nested then XContentMapValues.extractValue(...) extracts the values + // from two or more layers resulting in a list of list being returned. This is because nestedPath + // encapsulates two or more object layers in the _source. + // + // This is why only the first element of nestedParsedSource needs to be checked. + throw new IllegalArgumentException("Cannot execute inner hits. One or more parent object fields of nested field [" + + nestedObjectMapper.name() + "] are not nested. All parent fields need to be nested fields too"); + } + sourceAsMap = (Map) nestedParsedSource.get(nested.getOffset()); if (nested.getChild() == null) { current.put(nestedPath, sourceAsMap); } else { @@ -312,7 +323,8 @@ private Map getSearchFields(SearchContext context, int ne } private SearchHit.NestedIdentity getInternalNestedIdentity(SearchContext context, int nestedSubDocId, - LeafReaderContext subReaderContext, DocumentMapper documentMapper, + LeafReaderContext subReaderContext, + MapperService mapperService, ObjectMapper nestedObjectMapper) throws IOException { int currentParent = nestedSubDocId; ObjectMapper nestedParentObjectMapper; @@ -321,7 +333,7 @@ private SearchHit.NestedIdentity getInternalNestedIdentity(SearchContext context SearchHit.NestedIdentity nestedIdentity = null; do { Query parentFilter; - nestedParentObjectMapper = documentMapper.findParentObjectMapper(current); + nestedParentObjectMapper = current.getParentObjectMapper(mapperService); if (nestedParentObjectMapper != null) { if (nestedParentObjectMapper.nested().isNested() == false) { current = nestedParentObjectMapper; diff --git a/core/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java index 157033d414884..a3b477a4b6f25 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java @@ -36,6 +36,7 @@ import java.util.Collections; import java.util.function.Function; +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; @@ -428,4 +429,35 @@ public void testLimitOfNestedFieldsWithMultiTypePerIndex() throws Exception { createIndex("test5", Settings.builder().put(MapperService.INDEX_MAPPING_NESTED_FIELDS_LIMIT_SETTING.getKey(), 0).build()) .mapperService().merge("type", new CompressedXContent(mapping.apply("type")), MergeReason.MAPPING_RECOVERY, false); } + + public void testParentObjectMapperAreNested() throws Exception { + MapperService mapperService = createIndex("index1", Settings.EMPTY, "doc", jsonBuilder().startObject() + .startObject("properties") + .startObject("comments") + .field("type", "nested") + .startObject("properties") + .startObject("messages") + .field("type", "nested").endObject() + .endObject() + .endObject() + .endObject() + .endObject()).mapperService(); + ObjectMapper objectMapper = mapperService.getObjectMapper("comments.messages"); + assertTrue(objectMapper.parentObjectMapperAreNested(mapperService)); + + mapperService = createIndex("index2", Settings.EMPTY, "doc", jsonBuilder().startObject() + .startObject("properties") + .startObject("comments") + .field("type", "object") + .startObject("properties") + .startObject("messages") + .field("type", "nested").endObject() + .endObject() + .endObject() + .endObject() + .endObject()).mapperService(); + objectMapper = mapperService.getObjectMapper("comments.messages"); + assertFalse(objectMapper.parentObjectMapperAreNested(mapperService)); + } + } diff --git a/core/src/test/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java b/core/src/test/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java index 079db8097f2b0..e596ee206d19b 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java @@ -402,32 +402,54 @@ public void testInnerHitsWithObjectFieldThatHasANestedField() throws Exception { List requests = new ArrayList<>(); requests.add(client().prepareIndex("articles", "article", "1").setSource(jsonBuilder().startObject() .field("title", "quick brown fox") - .startObject("comments") - .startArray("messages") - .startObject().field("message", "fox eat quick").endObject() - .startObject().field("message", "bear eat quick").endObject() + .startArray("comments") + .startObject() + .startArray("messages") + .startObject().field("message", "fox eat quick").endObject() + .startObject().field("message", "bear eat quick").endObject() + .endArray() + .endObject() + .startObject() + .startArray("messages") + .startObject().field("message", "no fox").endObject() + .endArray() + .endObject() .endArray() - .endObject() .endObject())); indexRandom(true, requests); - SearchResponse response = client().prepareSearch("articles") + SearchResponse response = client().prepareSearch("articles").setQuery(nestedQuery("comments.messages", + matchQuery("comments.messages.message", "fox"), ScoreMode.Avg).innerHit(new InnerHitBuilder())).get(); + assertEquals("Cannot execute inner hits. One or more parent object fields of nested field [comments.messages] are " + + "not nested. All parent fields need to be nested fields too", response.getShardFailures()[0].getCause().getMessage()); + + response = client().prepareSearch("articles").setQuery(nestedQuery("comments.messages", + matchQuery("comments.messages.message", "fox"), ScoreMode.Avg).innerHit(new InnerHitBuilder() + .setFetchSourceContext(new FetchSourceContext(true)))).get(); + assertEquals("Cannot execute inner hits. One or more parent object fields of nested field [comments.messages] are " + + "not nested. All parent fields need to be nested fields too", response.getShardFailures()[0].getCause().getMessage()); + + response = client().prepareSearch("articles") .setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "fox"), ScoreMode.Avg) - .innerHit(new InnerHitBuilder())).get(); + .innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(false)))).get(); assertNoFailures(response); assertHitCount(response, 1); SearchHit hit = response.getHits().getAt(0); assertThat(hit.getId(), equalTo("1")); SearchHits messages = hit.getInnerHits().get("comments.messages"); - assertThat(messages.getTotalHits(), equalTo(1L)); + assertThat(messages.getTotalHits(), equalTo(2L)); assertThat(messages.getAt(0).getId(), equalTo("1")); assertThat(messages.getAt(0).getNestedIdentity().getField().string(), equalTo("comments.messages")); - assertThat(messages.getAt(0).getNestedIdentity().getOffset(), equalTo(0)); + assertThat(messages.getAt(0).getNestedIdentity().getOffset(), equalTo(2)); assertThat(messages.getAt(0).getNestedIdentity().getChild(), nullValue()); + assertThat(messages.getAt(1).getId(), equalTo("1")); + assertThat(messages.getAt(1).getNestedIdentity().getField().string(), equalTo("comments.messages")); + assertThat(messages.getAt(1).getNestedIdentity().getOffset(), equalTo(0)); + assertThat(messages.getAt(1).getNestedIdentity().getChild(), nullValue()); response = client().prepareSearch("articles") .setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "bear"), ScoreMode.Avg) - .innerHit(new InnerHitBuilder())).get(); + .innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(false)))).get(); assertNoFailures(response); assertHitCount(response, 1); hit = response.getHits().getAt(0); @@ -448,7 +470,7 @@ public void testInnerHitsWithObjectFieldThatHasANestedField() throws Exception { indexRandom(true, requests); response = client().prepareSearch("articles") .setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "fox"), ScoreMode.Avg) - .innerHit(new InnerHitBuilder())).get(); + .innerHit(new InnerHitBuilder().setFetchSourceContext(new FetchSourceContext(false)))).get(); assertNoFailures(response); assertHitCount(response, 1); hit = response.getHits().getAt(0);;