Skip to content

Commit

Permalink
inner_hits: Don't use bitset cache for children filters.
Browse files Browse the repository at this point in the history 
Only parent filters should use bitset filter cache, to avoid memory being wasted.
Also in case of object fields inline the field name into the nested object,
instead of creating an additional (dummy) nested identity.

Closes #10662
Closes #10629
  • Loading branch information
@martijnvg
martijnvg committed Apr 30, 2015
1 parent 82ad074 commit 7a6fe80
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 51 deletions.
46 changes: 22 additions & 24 deletions src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
View file
Expand Up @@ -24,8 +24,9 @@
import com.google.common.collect.Maps;
import org.apache.lucene.document.Field;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.search.DocIdSet;
import org.apache.lucene.search.DocIdSetIterator;
import org.apache.lucene.search.Filter;
import org.apache.lucene.util.BitDocIdSet;
import org.elasticsearch.ElasticsearchGenerationException;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
Expand All @@ -41,36 +42,19 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.cache.bitset.BitsetFilterCache;
import org.elasticsearch.index.mapper.Mapping.SourceTransform;
import org.elasticsearch.index.mapper.internal.AllFieldMapper;
import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper;
import org.elasticsearch.index.mapper.internal.IdFieldMapper;
import org.elasticsearch.index.mapper.internal.IndexFieldMapper;
import org.elasticsearch.index.mapper.internal.ParentFieldMapper;
import org.elasticsearch.index.mapper.internal.RoutingFieldMapper;
import org.elasticsearch.index.mapper.internal.SizeFieldMapper;
import org.elasticsearch.index.mapper.internal.SourceFieldMapper;
import org.elasticsearch.index.mapper.internal.TTLFieldMapper;
import org.elasticsearch.index.mapper.internal.TimestampFieldMapper;
import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
import org.elasticsearch.index.mapper.internal.UidFieldMapper;
import org.elasticsearch.index.mapper.internal.VersionFieldMapper;
import org.elasticsearch.index.mapper.internal.*;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import org.elasticsearch.index.mapper.object.RootObjectMapper;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.ScriptService.ScriptType;
import org.elasticsearch.search.internal.SearchContext;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.*;
import java.util.concurrent.CopyOnWriteArrayList;

/**
Expand Down Expand Up @@ -352,15 +336,29 @@ public ParsedDocument parse(SourceToParse source, @Nullable ParseListener listen
/**
* Returns the best nested {@link ObjectMapper} instances that is in the scope of the specified nested docId.
*/
public ObjectMapper findNestedObjectMapper(int nestedDocId, BitsetFilterCache cache, LeafReaderContext context) throws IOException {
public ObjectMapper findNestedObjectMapper(int nestedDocId, SearchContext sc, LeafReaderContext context) throws IOException {
ObjectMapper nestedObjectMapper = null;
for (ObjectMapper objectMapper : objectMappers().values()) {
if (!objectMapper.nested().isNested()) {
continue;
}

BitDocIdSet nestedTypeBitSet = cache.getBitDocIdSetFilter(objectMapper.nestedTypeFilter()).getDocIdSet(context);
if (nestedTypeBitSet != null && nestedTypeBitSet.bits().get(nestedDocId)) {
Filter filter = sc.filterCache().cache(objectMapper.nestedTypeFilter(), null, sc.queryParserService().autoFilterCachePolicy());
if (filter == null) {
continue;
}
// We can pass down 'null' as acceptedDocs, because nestedDocId is a doc to be fetched and
// therefor is guaranteed to be a live doc.
DocIdSet nestedTypeSet = filter.getDocIdSet(context, null);
if (nestedTypeSet == null) {
continue;
}
DocIdSetIterator iterator = nestedTypeSet.iterator();
if (iterator == null) {
continue;
}

if (iterator.advance(nestedDocId) == nestedDocId) {
if (nestedObjectMapper == null) {
nestedObjectMapper = objectMapper;
} else {
Expand Down
57 changes: 35 additions & 22 deletions src/main/java/org/elasticsearch/search/fetch/FetchPhase.java
View file
Expand Up @@ -21,9 +21,9 @@

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;

import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.index.ReaderUtil;
import org.apache.lucene.search.DocIdSet;
import org.apache.lucene.search.DocIdSetIterator;
import org.apache.lucene.search.Filter;
import org.apache.lucene.util.BitDocIdSet;
Expand Down Expand Up @@ -67,12 +67,7 @@
import org.elasticsearch.search.lookup.SourceLookup;

import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.*;

import static com.google.common.collect.Lists.newArrayList;
import static org.elasticsearch.common.xcontent.XContentFactory.contentBuilder;
Expand Down Expand Up @@ -288,7 +283,7 @@ private InternalSearchHit createNestedSearchHit(SearchContext context, int neste
SourceLookup sourceLookup = context.lookup().source();
sourceLookup.setSegmentAndDocument(subReaderContext, nestedSubDocId);

ObjectMapper nestedObjectMapper = documentMapper.findNestedObjectMapper(nestedSubDocId, context.bitsetFilterCache(), subReaderContext);
ObjectMapper nestedObjectMapper = documentMapper.findNestedObjectMapper(nestedSubDocId, context, subReaderContext);
assert nestedObjectMapper != null;
InternalSearchHit.InternalNestedIdentity nestedIdentity = getInternalNestedIdentity(context, nestedSubDocId, subReaderContext, documentMapper, nestedObjectMapper);

Expand Down Expand Up @@ -375,38 +370,56 @@ private Map<String, SearchHitField> getSearchFields(SearchContext context, int n
private InternalSearchHit.InternalNestedIdentity getInternalNestedIdentity(SearchContext context, int nestedSubDocId, LeafReaderContext subReaderContext, DocumentMapper documentMapper, ObjectMapper nestedObjectMapper) throws IOException {
int currentParent = nestedSubDocId;
ObjectMapper nestedParentObjectMapper;
StringBuilder field = new StringBuilder();
ObjectMapper current = nestedObjectMapper;
InternalSearchHit.InternalNestedIdentity nestedIdentity = null;
do {
String field;
Filter parentFilter;
nestedParentObjectMapper = documentMapper.findParentObjectMapper(nestedObjectMapper);
nestedParentObjectMapper = documentMapper.findParentObjectMapper(current);
if (field.length() != 0) {
field.insert(0, '.');
}
field.insert(0, current.name());
if (nestedParentObjectMapper != null) {
field = nestedObjectMapper.name();
if (!nestedParentObjectMapper.nested().isNested()) {
nestedObjectMapper = nestedParentObjectMapper;
// all right, the parent is a normal object field, so this is the best identiy we can give for that:
nestedIdentity = new InternalSearchHit.InternalNestedIdentity(field, 0, nestedIdentity);
if (nestedParentObjectMapper.nested().isNested() == false) {
current = nestedParentObjectMapper;
continue;
}
parentFilter = nestedParentObjectMapper.nestedTypeFilter();
} else {
field = nestedObjectMapper.fullPath();
parentFilter = Queries.newNonNestedFilter();
}

Filter childFilter = context.filterCache().cache(nestedObjectMapper.nestedTypeFilter(), null, context.queryParserService().autoFilterCachePolicy());
if (childFilter == null) {
current = nestedParentObjectMapper;
continue;
}
// We can pass down 'null' as acceptedDocs, because we're fetching matched docId that matched in the query phase.
DocIdSet childDocSet = childFilter.getDocIdSet(subReaderContext, null);
if (childDocSet == null) {
current = nestedParentObjectMapper;
continue;
}
DocIdSetIterator childIter = childDocSet.iterator();
if (childIter == null) {
current = nestedParentObjectMapper;
continue;
}

BitDocIdSet parentBitSet = context.bitsetFilterCache().getBitDocIdSetFilter(parentFilter).getDocIdSet(subReaderContext);
BitSet parentBits = parentBitSet.bits();

int offset = 0;
BitDocIdSet nestedDocsBitSet = context.bitsetFilterCache().getBitDocIdSetFilter(nestedObjectMapper.nestedTypeFilter()).getDocIdSet(subReaderContext);
BitSet nestedBits = nestedDocsBitSet.bits();
int nextParent = parentBits.nextSetBit(currentParent);
for (int docId = nestedBits.nextSetBit(currentParent + 1); docId < nextParent && docId != DocIdSetIterator.NO_MORE_DOCS; docId = nestedBits.nextSetBit(docId + 1)) {
for (int docId = childIter.advance(currentParent + 1); docId < nextParent && docId != DocIdSetIterator.NO_MORE_DOCS; docId = childIter.nextDoc()) {
offset++;
}
currentParent = nextParent;
nestedObjectMapper = nestedParentObjectMapper;
nestedIdentity = new InternalSearchHit.InternalNestedIdentity(field, offset, nestedIdentity);
} while (nestedParentObjectMapper != null);
current = nestedObjectMapper = nestedParentObjectMapper;
nestedIdentity = new InternalSearchHit.InternalNestedIdentity(field.toString(), offset, nestedIdentity);
field = new StringBuilder();
} while (current != null);
return nestedIdentity;
}

Expand Down
44 changes: 39 additions & 5 deletions src/test/java/org/elasticsearch/search/innerhits/InnerHitsTests.java
View file
Expand Up @@ -867,7 +867,12 @@ public void testInnerHitsWithObjectFieldThatHasANestedField() throws Exception {
List<IndexRequestBuilder> requests = new ArrayList<>();
requests.add(client().prepareIndex("articles", "article", "1").setSource(jsonBuilder().startObject()
.field("title", "quick brown fox")
.startObject("comments").startObject("messages").field("message", "fox eat quick").endObject().endObject()
.startObject("comments")
.startArray("messages")
.startObject().field("message", "fox eat quick").endObject()
.startObject().field("message", "bear eat quick").endObject()
.endArray()
.endObject()
.endObject()));
indexRandom(true, requests);

Expand All @@ -879,11 +884,40 @@ public void testInnerHitsWithObjectFieldThatHasANestedField() throws Exception {
assertThat(response.getHits().getAt(0).id(), equalTo("1"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getTotalHits(), equalTo(1l));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).id(), equalTo("1"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getField().string(), equalTo("comments"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getField().string(), equalTo("comments.messages"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getOffset(), equalTo(0));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getChild(), nullValue());

response = client().prepareSearch("articles")
.setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "bear")).innerHit(new QueryInnerHitBuilder()))
.get();
assertNoFailures(response);
assertHitCount(response, 1);
assertThat(response.getHits().getAt(0).id(), equalTo("1"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getTotalHits(), equalTo(1l));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).id(), equalTo("1"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getField().string(), equalTo("comments.messages"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getOffset(), equalTo(1));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getChild(), nullValue());

// index the message in an object form instead of an array
requests = new ArrayList<>();
requests.add(client().prepareIndex("articles", "article", "1").setSource(jsonBuilder().startObject()
.field("title", "quick brown fox")
.startObject("comments").startObject("messages").field("message", "fox eat quick").endObject().endObject()
.endObject()));
indexRandom(true, requests);
response = client().prepareSearch("articles")
.setQuery(nestedQuery("comments.messages", matchQuery("comments.messages.message", "fox")).innerHit(new QueryInnerHitBuilder()))
.get();
assertNoFailures(response);
assertHitCount(response, 1);
assertThat(response.getHits().getAt(0).id(), equalTo("1"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getTotalHits(), equalTo(1l));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).id(), equalTo("1"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getField().string(), equalTo("comments.messages"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getOffset(), equalTo(0));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getChild().getField().string(), equalTo("messages"));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getChild().getOffset(), equalTo(0));
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getChild().getChild(), nullValue());
assertThat(response.getHits().getAt(0).getInnerHits().get("comments.messages").getAt(0).getNestedIdentity().getChild(), nullValue());
}

}

0 comments on commit 7a6fe80

Please sign in to comment.