From 67c41d2e77f5e27ab546ab5d3519639fb833b371 Mon Sep 17 00:00:00 2001
From: Jim Ferenczi <jim.ferenczi@elastic.co>
Date: Wed, 17 May 2017 14:15:40 +0200
Subject: [PATCH] Fix ExpandSearchPhase when response contains no hits (#24688)

This change skips the expand search phase entirely when there is no search hits in the response.
---
 .../action/search/ExpandSearchPhase.java      |  2 +-
 .../action/search/ExpandSearchPhaseTests.java | 31 +++++++++++++++++++
 .../test/search/110_field_collapsing.yml      | 20 +++++++++++-
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java b/core/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java
index 1ec21ab4249..a8c5bdeacf3 100644
--- a/core/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java
+++ b/core/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java
@@ -64,7 +64,7 @@ final class ExpandSearchPhase extends SearchPhase {
 
     @Override
     public void run() throws IOException {
-        if (isCollapseRequest()) {
+        if (isCollapseRequest() && searchResponse.getHits().getHits().length > 0) {
             SearchRequest searchRequest = context.getRequest();
             CollapseBuilder collapseBuilder = searchRequest.source().collapse();
             MultiSearchRequest multiRequest = new MultiSearchRequest();
diff --git a/core/src/test/java/org/elasticsearch/action/search/ExpandSearchPhaseTests.java b/core/src/test/java/org/elasticsearch/action/search/ExpandSearchPhaseTests.java
index b7f0e0785f9..255025302c7 100644
--- a/core/src/test/java/org/elasticsearch/action/search/ExpandSearchPhaseTests.java
+++ b/core/src/test/java/org/elasticsearch/action/search/ExpandSearchPhaseTests.java
@@ -196,4 +196,35 @@ public class ExpandSearchPhaseTests extends ESTestCase {
         assertNotNull(reference.get());
         assertEquals(1, mockSearchPhaseContext.phasesExecuted.get());
     }
+
+    public void testSkipExpandCollapseNoHits() throws IOException {
+        MockSearchPhaseContext mockSearchPhaseContext = new MockSearchPhaseContext(1);
+        mockSearchPhaseContext.searchTransport = new SearchTransportService(
+            Settings.builder().put("search.remote.connect", false).build(), null) {
+
+            @Override
+            void sendExecuteMultiSearch(MultiSearchRequest request, SearchTask task, ActionListener<MultiSearchResponse> listener) {
+                fail("expand should not try to send empty multi search request");
+            }
+        };
+        mockSearchPhaseContext.getRequest().source(new SearchSourceBuilder()
+            .collapse(new CollapseBuilder("someField").setInnerHits(new InnerHitBuilder().setName("foobarbaz"))));
+
+        SearchHits hits = new SearchHits(new SearchHit[0], 1, 1.0f);
+        InternalSearchResponse internalSearchResponse = new InternalSearchResponse(hits, null, null, null, false, null, 1);
+        SearchResponse response = mockSearchPhaseContext.buildSearchResponse(internalSearchResponse, null);
+        AtomicReference<SearchResponse> reference = new AtomicReference<>();
+        ExpandSearchPhase phase = new ExpandSearchPhase(mockSearchPhaseContext, response, r ->
+            new SearchPhase("test") {
+                @Override
+                public void run() throws IOException {
+                    reference.set(r);
+                }
+            }
+        );
+        phase.run();
+        mockSearchPhaseContext.assertNoFailure();
+        assertNotNull(reference.get());
+        assertEquals(1, mockSearchPhaseContext.phasesExecuted.get());
+    }
 }
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml
index dd17399d31a..9d3fc349a23 100644
--- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml
+++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml
@@ -147,7 +147,6 @@ setup:
   - match: { hits.hits.2.inner_hits.sub_hits.hits.hits.0._id: "5" }
   - match: { hits.hits.2.inner_hits.sub_hits.hits.hits.1._id: "4" }
 
-
 ---
 "field collapsing, inner_hits and maxConcurrentGroupRequests":
 
@@ -247,3 +246,22 @@ setup:
                 match_all: {}
               query_weight: 1
               rescore_query_weight: 2
+
+---
+"no hits and inner_hits":
+
+  - skip:
+      version: " - 5.4.0"
+      reason:  "bug fixed in 5.4.1"
+
+  - do:
+      search:
+        index: test
+        type:  test
+        body:
+          size: 0
+          collapse: { field: numeric_group, inner_hits: { name: sub_hits, size: 1} }
+          sort: [{ sort: desc }]
+
+  - match: { hits.total: 6 }
+  - length: { hits.hits: 0 }