From d5aa35e0eafde434b6d7f93c3f347c44667cedd2 Mon Sep 17 00:00:00 2001
From: Martijn van Groningen <martijn.v.groningen@gmail.com>
Date: Tue, 18 Sep 2012 13:20:00 +0200
Subject: [PATCH] Added better error handling for has_child, has_parent and
 top_children.

If has_parent, has_child or top_children are executed incorrectly then a better exception is thrown. This gives a better error description when one of these queries or filters is being used in count api.

Closes #2261
---
 .../action/count/TransportCountAction.java    |  4 +-
 .../index/search/child/HasChildFilter.java    |  8 ++++
 .../index/search/child/HasParentFilter.java   |  8 ++++
 .../index/search/child/TopChildrenQuery.java  |  8 ++++
 .../child/SimpleChildQuerySearchTests.java    | 40 +++++++++++++++++++
 5 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/src/main/java/org/elasticsearch/action/count/TransportCountAction.java b/src/main/java/org/elasticsearch/action/count/TransportCountAction.java
index 21b946619d1..f97d4d91580 100644
--- a/src/main/java/org/elasticsearch/action/count/TransportCountAction.java
+++ b/src/main/java/org/elasticsearch/action/count/TransportCountAction.java
@@ -39,6 +39,7 @@ import org.elasticsearch.index.service.IndexService;
 import org.elasticsearch.index.shard.service.IndexShard;
 import org.elasticsearch.indices.IndicesService;
 import org.elasticsearch.script.ScriptService;
+import org.elasticsearch.search.SearchShardTarget;
 import org.elasticsearch.search.internal.InternalSearchRequest;
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.search.query.QueryPhaseExecutionException;
@@ -145,9 +146,10 @@ public class TransportCountAction extends TransportBroadcastOperationAction<Coun
         IndexService indexService = indicesService.indexServiceSafe(request.index());
         IndexShard indexShard = indexService.shardSafe(request.shardId());
 
+        SearchShardTarget shardTarget = new SearchShardTarget(clusterService.localNode().id(), request.index(), request.shardId());
         SearchContext context = new SearchContext(0,
                 new InternalSearchRequest().types(request.types()).filteringAliases(request.filteringAliases()),
-                null, indexShard.searcher(), indexService, indexShard,
+                shardTarget, indexShard.searcher(), indexService, indexShard,
                 scriptService);
         SearchContext.setCurrent(context);
 
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 f4106e9b38f..34862c1ba40 100644
--- a/src/main/java/org/elasticsearch/index/search/child/HasChildFilter.java
+++ b/src/main/java/org/elasticsearch/index/search/child/HasChildFilter.java
@@ -107,6 +107,10 @@ public abstract class HasChildFilter extends Filter implements ScopePhase.Collec
         }
 
         public DocIdSet getDocIdSet(IndexReader reader) throws IOException {
+            if (parentDocs == null) {
+                throw new ElasticSearchIllegalStateException("has_child filter/query hasn't executed properly");
+            }
+
             // ok to return null
             return parentDocs.get(reader.getCoreCacheKey());
         }
@@ -135,6 +139,10 @@ public abstract class HasChildFilter extends Filter implements ScopePhase.Collec
         }
 
         public DocIdSet getDocIdSet(IndexReader reader) throws IOException {
+            if (collectedUids == null) {
+                throw new ElasticSearchIllegalStateException("has_child filter/query hasn't executed properly");
+            }
+
             IdReaderTypeCache idReaderTypeCache = searchContext.idCache().reader(reader).type(parentType);
             return new ParentDocSet(reader, collectedUids, idReaderTypeCache);
         }
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 dc5a8d72bc1..adcdeb2d717 100644
--- a/src/main/java/org/elasticsearch/index/search/child/HasParentFilter.java
+++ b/src/main/java/org/elasticsearch/index/search/child/HasParentFilter.java
@@ -105,6 +105,10 @@ public abstract class HasParentFilter extends Filter implements ScopePhase.Colle
         }
 
         public DocIdSet getDocIdSet(IndexReader reader) throws IOException {
+            if (parents == null) {
+                throw new ElasticSearchIllegalStateException("has_parent filter/query hasn't executed properly");
+            }
+
             IdReaderTypeCache idReaderTypeCache = context.idCache().reader(reader).type(parentType);
             return new ChildrenDocSet(reader, parents, idReaderTypeCache);
         }
@@ -181,6 +185,10 @@ public abstract class HasParentFilter extends Filter implements ScopePhase.Colle
         }
 
         public DocIdSet getDocIdSet(IndexReader reader) throws IOException {
+            if (parentDocs == null) {
+                throw new ElasticSearchIllegalStateException("has_parent filter/query hasn't executed properly");
+            }
+
             return new ChildrenDocSet(reader, parentDocs, context, parentType);
         }
 
diff --git a/src/main/java/org/elasticsearch/index/search/child/TopChildrenQuery.java b/src/main/java/org/elasticsearch/index/search/child/TopChildrenQuery.java
index e5d3fdaa02b..47a10f42d3e 100644
--- a/src/main/java/org/elasticsearch/index/search/child/TopChildrenQuery.java
+++ b/src/main/java/org/elasticsearch/index/search/child/TopChildrenQuery.java
@@ -74,6 +74,9 @@ public class TopChildrenQuery extends Query implements ScopePhase.TopDocsPhase {
 
     private int numHits = 0;
 
+    // Need to know if this query is properly used, otherwise the results are unexpected for example in the count api
+    private boolean properlyInvoked = false;
+
     // Note, the query is expected to already be filtered to only child type docs
     public TopChildrenQuery(Query query, String scope, String childType, String parentType, ScoreType scoreType, int factor, int incrementalFactor) {
         this.query = query;
@@ -97,6 +100,7 @@ public class TopChildrenQuery extends Query implements ScopePhase.TopDocsPhase {
 
     @Override
     public void clear() {
+        properlyInvoked = true;
         parentDocs = null;
         numHits = 0;
     }
@@ -202,6 +206,10 @@ public class TopChildrenQuery extends Query implements ScopePhase.TopDocsPhase {
 
     @Override
     public Weight createWeight(Searcher searcher) throws IOException {
+        if (!properlyInvoked) {
+            throw new ElasticSearchIllegalStateException("top_children query hasn't executed properly");
+        }
+
         if (parentDocs != null) {
             return new ParentWeight(searcher, query.weight(searcher));
         }
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 53b7f912564..85be6e89a66 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
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.test.integration.search.child;
 
+import org.elasticsearch.action.count.CountResponse;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.search.SearchType;
 import org.elasticsearch.action.search.ShardSearchFailure;
@@ -705,4 +706,43 @@ public class SimpleChildQuerySearchTests extends AbstractNodesTests {
         assertThat(searchResponse.hits().getAt(0).id(), equalTo("p2"));
     }
 
+    @Test
+    public void testCountApiUsage() throws Exception {
+        client.admin().indices().prepareDelete().execute().actionGet();
+
+        client.admin().indices().prepareCreate("test").setSettings(ImmutableSettings.settingsBuilder().put("index.number_of_shards", 1)).execute().actionGet();
+        client.admin().cluster().prepareHealth().setWaitForGreenStatus().execute().actionGet();
+        client.admin().indices().preparePutMapping("test").setType("child").setSource(XContentFactory.jsonBuilder().startObject().startObject("type")
+                .startObject("_parent").field("type", "parent").endObject()
+                .endObject().endObject()).execute().actionGet();
+
+        String parentId = "p1";
+        client.prepareIndex("test", "parent", parentId)
+                .setSource("p_field", "p_value1")
+                .execute().actionGet();
+        client.prepareIndex("test", "child", parentId + "_c1")
+                .setSource("c_field1", parentId)
+                .setParent(parentId)
+                .execute().actionGet();
+        client.admin().indices().prepareRefresh().execute().actionGet();
+
+        CountResponse countResponse = client.prepareCount("test")
+                .setQuery(topChildrenQuery("child", termQuery("c_field1", "1")))
+                .execute().actionGet();
+        assertThat(countResponse.failedShards(), equalTo(1));
+        assertThat(countResponse.shardFailures().get(0).reason().contains("top_children query hasn't executed properly"), equalTo(true));
+
+        countResponse = client.prepareCount("test")
+                .setQuery(hasChildQuery("child", termQuery("c_field1", "2")).executionType(getExecutionMethod()))
+                .execute().actionGet();
+        assertThat(countResponse.failedShards(), equalTo(1));
+        assertThat(countResponse.shardFailures().get(0).reason().contains("has_child filter/query hasn't executed properly"), equalTo(true));
+
+        countResponse = client.prepareCount("test")
+                .setQuery(hasParentQuery("parent", termQuery("p_field1", "1")).executionType(getExecutionMethod()))
+                .execute().actionGet();
+        assertThat(countResponse.failedShards(), equalTo(1));
+        assertThat(countResponse.shardFailures().get(0).reason().contains("has_parent filter/query hasn't executed properly"), equalTo(true));
+    }
+
 }