From 1ce10dfb06fe5e90330619b881ce416e8bdf6d9a Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Fri, 11 Jan 2013 16:06:14 +0100 Subject: [PATCH] Fixed issue where parent & child queries can fail if a segment doesn't have documents with the targeted type or associated parent type Closes #2537 --- .../index/query/HasChildQueryBuilder.java | 4 +- .../index/search/child/ChildCollector.java | 6 +- .../index/search/child/ChildrenQuery.java | 8 +++ .../index/search/child/HasChildFilter.java | 6 +- .../index/search/child/HasParentFilter.java | 18 ++++-- .../index/search/child/ParentQuery.java | 4 ++ .../child/SimpleChildQuerySearchTests.java | 61 +++++++++++++++++++ 7 files changed, 97 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java b/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java index d4dba5cc6fa..e7b41e3a832 100644 --- a/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java +++ b/src/main/java/org/elasticsearch/index/query/HasChildQueryBuilder.java @@ -66,8 +66,8 @@ public class HasChildQueryBuilder extends BaseQueryBuilder implements BoostableQ /** * Defines how the scores from the matching child documents are mapped into the parent document. */ - public HasChildQueryBuilder scoreType(String executionType) { - this.scoreType = executionType; + public HasChildQueryBuilder scoreType(String scoreType) { + this.scoreType = scoreType; return this; } diff --git a/src/main/java/org/elasticsearch/index/search/child/ChildCollector.java b/src/main/java/org/elasticsearch/index/search/child/ChildCollector.java index 33e7366668e..9047a99866c 100644 --- a/src/main/java/org/elasticsearch/index/search/child/ChildCollector.java +++ b/src/main/java/org/elasticsearch/index/search/child/ChildCollector.java @@ -21,7 +21,6 @@ package org.elasticsearch.index.search.child; import org.apache.lucene.index.AtomicReader; import org.apache.lucene.index.AtomicReaderContext; -import org.apache.lucene.index.IndexReader; import org.apache.lucene.search.Collector; import org.apache.lucene.search.Scorer; import org.apache.lucene.util.FixedBitSet; @@ -70,11 +69,14 @@ public class ChildCollector extends Collector { @Override public void setScorer(Scorer scorer) throws IOException { - } @Override public void collect(int doc) throws IOException { + if (typeCache == null) { + return; + } + HashedBytesArray parentId = typeCache.parentIdByDoc(doc); if (parentId == null) { return; diff --git a/src/main/java/org/elasticsearch/index/search/child/ChildrenQuery.java b/src/main/java/org/elasticsearch/index/search/child/ChildrenQuery.java index 2fed97e1ba2..f05f432d31c 100644 --- a/src/main/java/org/elasticsearch/index/search/child/ChildrenQuery.java +++ b/src/main/java/org/elasticsearch/index/search/child/ChildrenQuery.java @@ -321,6 +321,10 @@ public class ChildrenQuery extends Query implements ScopePhase.CollectorPhase { @Override public void collect(int doc) throws IOException { + if (typeCache == null) { + return; + } + HashedBytesArray parentUid = typeCache.parentIdByDoc(doc); float previousScore = uidToScore.get(parentUid); float currentScore = scorer.score(); @@ -364,6 +368,10 @@ public class ChildrenQuery extends Query implements ScopePhase.CollectorPhase { @Override public void collect(int doc) throws IOException { + if (typeCache == null) { + return; + } + HashedBytesArray parentUid = typeCache.parentIdByDoc(doc); float previousScore = uidToScore.get(parentUid); float currentScore = scorer.score(); diff --git a/src/main/java/org/elasticsearch/index/search/child/HasChildFilter.java b/src/main/java/org/elasticsearch/index/search/child/HasChildFilter.java index ed07be5634d..52f40e0978c 100644 --- a/src/main/java/org/elasticsearch/index/search/child/HasChildFilter.java +++ b/src/main/java/org/elasticsearch/index/search/child/HasChildFilter.java @@ -198,7 +198,11 @@ public abstract class HasChildFilter extends Filter implements ScopePhase.Collec @Override public void collect(int doc) throws IOException { - collectedUids.add(typeCache.parentIdByDoc(doc)); + // It can happen that for particular segment no document exist for an specific type. This prevents NPE + if (typeCache != null) { + collectedUids.add(typeCache.parentIdByDoc(doc)); + } + } @Override diff --git a/src/main/java/org/elasticsearch/index/search/child/HasParentFilter.java b/src/main/java/org/elasticsearch/index/search/child/HasParentFilter.java index a032d2af171..a35e58ac79f 100644 --- a/src/main/java/org/elasticsearch/index/search/child/HasParentFilter.java +++ b/src/main/java/org/elasticsearch/index/search/child/HasParentFilter.java @@ -163,7 +163,10 @@ public abstract class HasParentFilter extends Filter implements ScopePhase.Colle } public void collect(int doc) throws IOException { - collectedUids.add(typeCache.idByDoc(doc)); + // It can happen that for particular segment no document exist for an specific type. This prevents NPE + if (typeCache != null) { + collectedUids.add(typeCache.idByDoc(doc)); + } } @Override @@ -199,7 +202,12 @@ public abstract class HasParentFilter extends Filter implements ScopePhase.Colle throw new ElasticSearchIllegalStateException("has_parent filter hasn't executed properly"); } - return new ChildrenDocSet(readerContext.reader(), acceptDocs, parentDocs, context, parentType); + IdReaderTypeCache currentTypeCache = context.idCache().reader(readerContext.reader()).type(parentType); + if (currentTypeCache == null) { + return null; + } else { + return new ChildrenDocSet(readerContext.reader(), currentTypeCache, acceptDocs, parentDocs, context, parentType); + } } public void clear() { @@ -213,10 +221,10 @@ public abstract class HasParentFilter extends Filter implements ScopePhase.Colle final Tuple[] readersToTypeCache; final Map parentDocs; - ChildrenDocSet(AtomicReader currentReader, @Nullable Bits acceptDocs, Map parentDocs, - SearchContext context, String parentType) { + ChildrenDocSet(AtomicReader currentReader, IdReaderTypeCache currentTypeCache, @Nullable Bits acceptDocs, + Map parentDocs, SearchContext context, String parentType) { super(currentReader.maxDoc(), acceptDocs); - this.currentTypeCache = context.idCache().reader(currentReader).type(parentType); + this.currentTypeCache = currentTypeCache; this.currentReader = currentReader; this.parentDocs = parentDocs; this.readersToTypeCache = new Tuple[context.searcher().getIndexReader().leaves().size()]; diff --git a/src/main/java/org/elasticsearch/index/search/child/ParentQuery.java b/src/main/java/org/elasticsearch/index/search/child/ParentQuery.java index 7dd97674880..d625dd12365 100644 --- a/src/main/java/org/elasticsearch/index/search/child/ParentQuery.java +++ b/src/main/java/org/elasticsearch/index/search/child/ParentQuery.java @@ -160,6 +160,10 @@ public class ParentQuery extends Query implements ScopePhase.CollectorPhase { @Override public void collect(int doc) throws IOException { + if (typeCache == null) { + return; + } + HashedBytesArray parentUid = typeCache.idByDoc(doc); uidToScore.put(parentUid, scorer.score()); } diff --git a/src/test/java/org/elasticsearch/test/integration/search/child/SimpleChildQuerySearchTests.java b/src/test/java/org/elasticsearch/test/integration/search/child/SimpleChildQuerySearchTests.java index f19e3174617..8c3494850da 100644 --- a/src/test/java/org/elasticsearch/test/integration/search/child/SimpleChildQuerySearchTests.java +++ b/src/test/java/org/elasticsearch/test/integration/search/child/SimpleChildQuerySearchTests.java @@ -1077,4 +1077,65 @@ public class SimpleChildQuerySearchTests extends AbstractNodesTests { assertThat(response.hits().hits()[6].score(), equalTo(5f)); } + @Test + // https://github.com/elasticsearch/elasticsearch/issues/2536 + public void testParentChildQueriesCanHandleNoRelevantTypesInIndex() throws Exception { + client.admin().indices().prepareDelete().execute().actionGet(); + + client.admin().indices().prepareCreate("test") + .addMapping("parent", jsonBuilder() + .startObject() + .startObject("parent") + .endObject() + .endObject() + ).addMapping("child", jsonBuilder() + .startObject() + .startObject("child") + .startObject("_parent") + .field("type", "parent") + .endObject() + .endObject() + .endObject() + ).setSettings( + ImmutableSettings.settingsBuilder() + .put("index.number_of_shards", 1) + .put("index.number_of_replicas", 0) + ).execute().actionGet(); + client.admin().cluster().prepareHealth().setWaitForGreenStatus().execute().actionGet(); + + SearchResponse response = client.prepareSearch("test") + .setQuery(QueryBuilders.hasChildQuery("child", matchQuery("text", "value"))) + .execute().actionGet(); + assertThat(response.failedShards(), equalTo(0)); + assertThat(response.hits().totalHits(), equalTo(0l)); + + client.prepareIndex("test", "child1").setSource(jsonBuilder().startObject().field("text", "value").endObject()) + .setRefresh(true) + .execute().actionGet(); + + client.prepareSearch("test") + .setQuery(QueryBuilders.hasChildQuery("child", matchQuery("text", "value")).executionType(getExecutionMethod())) + .execute().actionGet(); + assertThat(response.failedShards(), equalTo(0)); + assertThat(response.hits().totalHits(), equalTo(0l)); + + client.prepareSearch("test") + .setQuery(QueryBuilders.hasChildQuery("child", matchQuery("text", "value")).scoreType("max")) + .execute().actionGet(); + assertThat(response.failedShards(), equalTo(0)); + assertThat(response.hits().totalHits(), equalTo(0l)); + + client.prepareSearch("test") + .setQuery(QueryBuilders.hasParentQuery("child", matchQuery("text", "value")).executionType(getExecutionMethod())) + .execute().actionGet(); + assertThat(response.failedShards(), equalTo(0)); + assertThat(response.hits().totalHits(), equalTo(0l)); + + client.prepareSearch("test") + .setQuery(QueryBuilders.hasParentQuery("child", matchQuery("text", "value")).scoreType("score")) + .execute().actionGet(); + assertThat(response.failedShards(), equalTo(0)); + assertThat(response.hits().totalHits(), equalTo(0l)); + } + }