Skip to content

Commit

Permalink
percolator: fix handling of nested documents
Browse files Browse the repository at this point in the history 
Nested documents were indexed as separate documents, but it was never checked
if the hits represent nested documents or not. Therefore, nested objects could
match not nested queries and nested queries could also match not nested documents.

Examples are in issue elastic#6540 .

closes elastic#6540
  • Loading branch information
@brwe
brwe committed Jun 18, 2014
1 parent 3eb291f commit 269fe71
Show file tree
Hide file tree
Showing 2 changed files with 152 additions and 18 deletions.
49 changes: 31 additions & 18 deletions src/main/java/org/elasticsearch/percolator/PercolatorService.java
View file
Expand Up @@ -68,6 +68,7 @@
import org.elasticsearch.index.mapper.internal.IdFieldMapper;
import org.elasticsearch.index.percolator.stats.ShardPercolateService;
import org.elasticsearch.index.query.ParsedQuery;
import org.elasticsearch.index.search.nested.NonNestedDocsFilter;
import org.elasticsearch.index.service.IndexService;
import org.elasticsearch.index.shard.service.IndexShard;
import org.elasticsearch.indices.IndicesService;
Expand Down Expand Up @@ -210,8 +211,9 @@ public PercolateShardResponse percolate(PercolateShardRequest request) {

// parse the source either into one MemoryIndex, if it is a single document or index multiple docs if nested
PercolatorIndex percolatorIndex;
boolean isNested = indexShard.mapperService().documentMapper(request.documentType()).hasNestedObjects();
if (parsedDocument.docs().size() > 1) {
assert indexShard.mapperService().documentMapper(request.documentType()).hasNestedObjects();
assert isNested;
percolatorIndex = multi;
} else {
percolatorIndex = single;
Expand All @@ -232,7 +234,7 @@ public PercolateShardResponse percolate(PercolateShardRequest request) {
context.percolatorTypeId = action.id();

percolatorIndex.prepare(context, parsedDocument);
return action.doPercolate(request, context);
return action.doPercolate(request, context, isNested);
} finally {
context.close();
shardPercolateService.postPercolate(System.nanoTime() - startTime);
Expand Down Expand Up @@ -418,7 +420,7 @@ interface PercolatorType {

ReduceResult reduce(List<PercolateShardResponse> shardResults);

PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context);
PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context, boolean isNested);

}

Expand All @@ -443,13 +445,17 @@ public ReduceResult reduce(List<PercolateShardResponse> shardResults) {
}

@Override
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context) {
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context, boolean isNested) {
long count = 0;
Lucene.ExistsCollector collector = new Lucene.ExistsCollector();
for (Map.Entry<BytesRef, Query> entry : context.percolateQueries().entrySet()) {
collector.reset();
try {
context.docSearcher().search(entry.getValue(), collector);
if (isNested) {
context.docSearcher().search(entry.getValue(), NonNestedDocsFilter.INSTANCE, collector);
} else {
context.docSearcher().search(entry.getValue(), collector);
}
} catch (Throwable e) {
logger.debug("[" + entry.getKey() + "] failed to execute query", e);
throw new PercolateException(context.indexShard().shardId(), "failed to execute", e);
Expand Down Expand Up @@ -477,12 +483,12 @@ public ReduceResult reduce(List<PercolateShardResponse> shardResults) {
}

@Override
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context) {
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context, boolean isNested) {
long count = 0;
Engine.Searcher percolatorSearcher = context.indexShard().acquireSearcher("percolate");
try {
Count countCollector = count(logger, context);
queryBasedPercolating(percolatorSearcher, context, countCollector);
queryBasedPercolating(percolatorSearcher, context, countCollector, isNested);
count = countCollector.counter();
} catch (Throwable e) {
logger.warn("failed to execute", e);
Expand Down Expand Up @@ -534,7 +540,7 @@ public ReduceResult reduce(List<PercolateShardResponse> shardResults) {
}

@Override
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context) {
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context, boolean isNested) {
long count = 0;
List<BytesRef> matches = new ArrayList<>();
List<Map<String, HighlightField>> hls = new ArrayList<>();
Expand All @@ -547,7 +553,11 @@ public PercolateShardResponse doPercolate(PercolateShardRequest request, Percola
context.hitContext().cache().clear();
}
try {
context.docSearcher().search(entry.getValue(), collector);
if (isNested) {
context.docSearcher().search(entry.getValue(), NonNestedDocsFilter.INSTANCE, collector);
} else {
context.docSearcher().search(entry.getValue(), collector);
}
} catch (Throwable e) {
logger.debug("[" + entry.getKey() + "] failed to execute query", e);
throw new PercolateException(context.indexShard().shardId(), "failed to execute", e);
Expand Down Expand Up @@ -583,11 +593,11 @@ public ReduceResult reduce(List<PercolateShardResponse> shardResults) {
}

@Override
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context) {
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context, boolean isNested) {
Engine.Searcher percolatorSearcher = context.indexShard().acquireSearcher("percolate");
try {
Match match = match(logger, context, highlightPhase);
queryBasedPercolating(percolatorSearcher, context, match);
queryBasedPercolating(percolatorSearcher, context, match, isNested);
List<BytesRef> matches = match.matches();
List<Map<String, HighlightField>> hls = match.hls();
long count = match.counter();
Expand Down Expand Up @@ -616,11 +626,11 @@ public ReduceResult reduce(List<PercolateShardResponse> shardResults) {
}

@Override
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context) {
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context, boolean isNested) {
Engine.Searcher percolatorSearcher = context.indexShard().acquireSearcher("percolate");
try {
MatchAndScore matchAndScore = matchAndScore(logger, context, highlightPhase);
queryBasedPercolating(percolatorSearcher, context, matchAndScore);
queryBasedPercolating(percolatorSearcher, context, matchAndScore, isNested);
List<BytesRef> matches = matchAndScore.matches();
List<Map<String, HighlightField>> hls = matchAndScore.hls();
float[] scores = matchAndScore.scores().toArray();
Expand Down Expand Up @@ -730,11 +740,11 @@ public ReduceResult reduce(List<PercolateShardResponse> shardResults) {
}

@Override
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context) {
public PercolateShardResponse doPercolate(PercolateShardRequest request, PercolateContext context, boolean isNested) {
Engine.Searcher percolatorSearcher = context.indexShard().acquireSearcher("percolate");
try {
MatchAndSort matchAndSort = QueryCollector.matchAndSort(logger, context);
queryBasedPercolating(percolatorSearcher, context, matchAndSort);
queryBasedPercolating(percolatorSearcher, context, matchAndSort, isNested);
TopDocs topDocs = matchAndSort.topDocs();
long count = topDocs.totalHits;
List<BytesRef> matches = new ArrayList<>(topDocs.scoreDocs.length);
Expand Down Expand Up @@ -780,12 +790,15 @@ public PercolateShardResponse doPercolate(PercolateShardRequest request, Percola

};

private void queryBasedPercolating(Engine.Searcher percolatorSearcher, PercolateContext context, QueryCollector percolateCollector) throws IOException {
private void queryBasedPercolating(Engine.Searcher percolatorSearcher, PercolateContext context, QueryCollector percolateCollector, boolean isNested) throws IOException {
Filter percolatorTypeFilter = context.indexService().mapperService().documentMapper(TYPE_NAME).typeFilter();
percolatorTypeFilter = context.indexService().cache().filter().cache(percolatorTypeFilter);
XFilteredQuery query = new XFilteredQuery(context.percolateQuery(), percolatorTypeFilter);
percolatorSearcher.searcher().search(query, percolateCollector);

if (isNested) {
percolatorSearcher.searcher().search(query, NonNestedDocsFilter.INSTANCE, percolateCollector);
} else {
percolatorSearcher.searcher().search(query, percolateCollector);
}
for (Collector queryCollector : percolateCollector.facetAndAggregatorCollector) {
if (queryCollector instanceof XCollector) {
((XCollector) queryCollector).postCollection();
Expand Down
121 changes: 121 additions & 0 deletions src/test/java/org/elasticsearch/percolator/PercolatorTests.java
View file
Expand Up @@ -61,6 +61,7 @@
import static org.elasticsearch.common.xcontent.XContentFactory.*;
import static org.elasticsearch.index.query.FilterBuilders.termFilter;
import static org.elasticsearch.index.query.QueryBuilders.*;
import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.exponentialDecayFunction;
import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.scriptFunction;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*;
import static org.hamcrest.Matchers.*;
Expand Down Expand Up @@ -1843,4 +1844,124 @@ XContentBuilder getNotMatchingNestedDoc() throws IOException {
.endArray().endObject();
return doc;
}

// issue
@Test
public void testNestedDocFilter() throws IOException {
String mapping = "{\n" +
" \"doc\": {\n" +
" \"properties\": {\n" +
" \"persons\": {\n" +
" \"type\": \"nested\"\n" +
" }\n" +
" }\n" +
" }\n" +
" }";
String doc = "{\n" +
" \"name\": \"obama\",\n" +
" \"persons\": [\n" +
" {\n" +
" \"foo\": \"bar\"\n" +
" }\n" +
" ]\n" +
" }";
String q1 = "{\n" +
" \"query\": {\n" +
" \"bool\": {\n" +
" \"must\": {\n" +
" \"match\": {\n" +
" \"name\": \"obama\"\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
"}";
String q2 = "{\n" +
" \"query\": {\n" +
" \"bool\": {\n" +
" \"must_not\": {\n" +
" \"match\": {\n" +
" \"name\": \"obama\"\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
"}";
String q3 = "{\n" +
" \"query\": {\n" +
" \"bool\": {\n" +
" \"must\": {\n" +
" \"match\": {\n" +
" \"persons.foo\": \"bar\"\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
"}";
String q4 = "{\n" +
" \"query\": {\n" +
" \"bool\": {\n" +
" \"must_not\": {\n" +
" \"match\": {\n" +
" \"persons.foo\": \"bar\"\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
"}";
String q5 = "{\n" +
" \"query\": {\n" +
" \"bool\": {\n" +
" \"must\": {\n" +
" \"nested\": {\n" +
" \"path\": \"persons\",\n" +
" \"query\": {\n" +
" \"match\": {\n" +
" \"persons.foo\": \"bar\"\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
"}";
String q6 = "{\n" +
" \"query\": {\n" +
" \"bool\": {\n" +
" \"must_not\": {\n" +
" \"nested\": {\n" +
" \"path\": \"persons\",\n" +
" \"query\": {\n" +
" \"match\": {\n" +
" \"persons.foo\": \"bar\"\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
" }\n" +
"}";
assertAcked(client().admin().indices().prepareCreate("test").addMapping("doc", mapping));
ensureGreen("test");
client().prepareIndex("test", PercolatorService.TYPE_NAME).setSource(q1).setId("q1").get();
client().prepareIndex("test", PercolatorService.TYPE_NAME).setSource(q2).setId("q2").get();
client().prepareIndex("test", PercolatorService.TYPE_NAME).setSource(q3).setId("q3").get();
client().prepareIndex("test", PercolatorService.TYPE_NAME).setSource(q4).setId("q4").get();
client().prepareIndex("test", PercolatorService.TYPE_NAME).setSource(q5).setId("q5").get();
client().prepareIndex("test", PercolatorService.TYPE_NAME).setSource(q6).setId("q6").get();
PercolateResponse response = client().preparePercolate()
.setIndices("test").setDocumentType("doc")
.setPercolateDoc(docBuilder().setDoc(doc))
.get();
assertMatchCount(response, 3l);
Set<String> expectedIds = new HashSet<>();
expectedIds.add("q1");
expectedIds.add("q4");
expectedIds.add("q5");
for (PercolateResponse.Match match : response.getMatches()) {
assertTrue(expectedIds.remove(match.getId().string()));
}
assertTrue(expectedIds.isEmpty());
}
}

0 comments on commit 269fe71

Please sign in to comment.