Simplify SubFetchPhase interface (#18881)

This interface used to have dedicated methods to prevent calling execute
methods. These methods are unnecessary as the checks can simply be
done inside the execute methods itself. This simplifies the interface
as well as its usage.

core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java

@@ -160,16 +160,12 @@ public class FetchPhase implements SearchPhase {
             hits[index] = searchHit;
             hitContext.reset(searchHit, subReaderContext, subDocId, context.searcher());
             for (FetchSubPhase fetchSubPhase : fetchSubPhases) {
-                if (fetchSubPhase.hitExecutionNeeded(context)) {
-                    fetchSubPhase.hitExecute(context, hitContext);
-                }
+                fetchSubPhase.hitExecute(context, hitContext);
             }
         }
 
         for (FetchSubPhase fetchSubPhase : fetchSubPhases) {
-            if (fetchSubPhase.hitsExecutionNeeded(context)) {
-                fetchSubPhase.hitsExecute(context, hits);
-            }
+            fetchSubPhase.hitsExecute(context, hits);
         }
 
         context.fetchResult().hits(new InternalSearchHits(hits, context.queryResult().topDocs().totalHits, context.queryResult().topDocs().getMaxScore()));

core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java

@@ -36,7 +36,7 @@ import java.util.Map;
  */
 public interface FetchSubPhase {
 
-    public static class HitContext {
+    class HitContext {
         private InternalSearchHit hit;
         private IndexSearcher searcher;
         private LeafReaderContext readerContext;
@@ -87,16 +87,13 @@ public interface FetchSubPhase {
         return Collections.emptyMap();
     }
 
-    boolean hitExecutionNeeded(SearchContext context);
-
     /**
      * Executes the hit level phase, with a reader and doc id (note, its a low level reader, and the matching doc).
      */
-    void hitExecute(SearchContext context, HitContext hitContext);
+    default void hitExecute(SearchContext context, HitContext hitContext) {}
 
-    boolean hitsExecutionNeeded(SearchContext context);
 
-    void hitsExecute(SearchContext context, InternalSearchHit[] hits);
+    default void hitsExecute(SearchContext context, InternalSearchHit[] hits) {}
 
     /**
      * This interface is in the fetch phase plugin mechanism.
@@ -104,16 +101,16 @@ public interface FetchSubPhase {
      * Fetch phases that use the plugin mechanism must provide a ContextFactory to the SearchContext that creates the fetch phase context and also associates them with a name.
      * See {@link SearchContext#getFetchSubPhaseContext(FetchSubPhase.ContextFactory)}
      */
-    public interface ContextFactory<SubPhaseContext extends FetchSubPhaseContext> {
+    interface ContextFactory<SubPhaseContext extends FetchSubPhaseContext> {
 
         /**
          * The name of the context.
          */
-        public String getName();
+        String getName();
 
         /**
          * Creates a new instance of a FetchSubPhaseContext that holds all information a FetchSubPhase needs to execute on hits.
          */
-        public SubPhaseContext newContextInstance();
+        SubPhaseContext newContextInstance();
     }
 }

core/src/main/java/org/elasticsearch/search/fetch/explain/ExplainFetchSubPhase.java

@@ -30,24 +30,13 @@ import java.io.IOException;
 /**
  *
  */
-public class ExplainFetchSubPhase implements FetchSubPhase {
-
-    @Override
-    public boolean hitsExecutionNeeded(SearchContext context) {
-        return false;
-    }
-
-    @Override
-    public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
-    }
-
-    @Override
-    public boolean hitExecutionNeeded(SearchContext context) {
-        return context.explain();
-    }
+public final class ExplainFetchSubPhase implements FetchSubPhase {
 
     @Override
     public void hitExecute(SearchContext context, HitContext hitContext) {
+        if (context.explain() == false) {
+            return;
+        }
         try {
             final int topLevelDocId = hitContext.hit().docId();
             Explanation explanation = context.searcher().explain(context.query(), topLevelDocId);

core/src/main/java/org/elasticsearch/search/fetch/fielddata/FieldDataFieldsFetchSubPhase.java

@@ -18,13 +18,11 @@
  */
 package org.elasticsearch.search.fetch.fielddata;
 
-import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.index.fielddata.AtomicFieldData;
 import org.elasticsearch.index.fielddata.ScriptDocValues;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.search.SearchHitField;
 import org.elasticsearch.search.fetch.FetchSubPhase;
-import org.elasticsearch.search.internal.InternalSearchHit;
 import org.elasticsearch.search.internal.InternalSearchHitField;
 import org.elasticsearch.search.internal.SearchContext;
 
@@ -37,7 +35,7 @@ import java.util.HashMap;
  *
  * Specifying {@code "fielddata_fields": ["field1", "field2"]}
  */
-public class FieldDataFieldsFetchSubPhase implements FetchSubPhase {
+public final class FieldDataFieldsFetchSubPhase implements FetchSubPhase {
 
     public static final String[] NAMES = {"fielddata_fields", "fielddataFields"};
     public static final ContextFactory<FieldDataFieldsContext> CONTEXT_FACTORY = new ContextFactory<FieldDataFieldsContext>() {
@@ -53,29 +51,14 @@ public class FieldDataFieldsFetchSubPhase implements FetchSubPhase {
         }
     };
 
-    @Inject
-    public FieldDataFieldsFetchSubPhase() {
-    }
-
-    @Override
-    public boolean hitsExecutionNeeded(SearchContext context) {
-        return false;
-    }
-
-    @Override
-    public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
-    }
-
-    @Override
-    public boolean hitExecutionNeeded(SearchContext context) {
-        return context.getFetchSubPhaseContext(CONTEXT_FACTORY).hitExecutionNeeded();
-    }
-
     @Override
     public void hitExecute(SearchContext context, HitContext hitContext) {
+        if (context.getFetchSubPhaseContext(CONTEXT_FACTORY).hitExecutionNeeded() == false) {
+            return;
+        }
         for (FieldDataFieldsContext.FieldDataField field : context.getFetchSubPhaseContext(CONTEXT_FACTORY).fields()) {
             if (hitContext.hit().fieldsOrNull() == null) {
-                hitContext.hit().fields(new HashMap<String, SearchHitField>(2));
+                hitContext.hit().fields(new HashMap<>(2));
             }
             SearchHitField hitField = hitContext.hit().fields().get(field.name());
             if (hitField == null) {

core/src/main/java/org/elasticsearch/search/fetch/innerhits/InnerHitsFetchSubPhase.java

@@ -23,7 +23,6 @@ import org.apache.lucene.search.FieldDoc;
 import org.apache.lucene.search.ScoreDoc;
 import org.apache.lucene.search.TopDocs;
 import org.elasticsearch.ExceptionsHelper;
-import org.elasticsearch.search.SearchParseElement;
 import org.elasticsearch.search.fetch.FetchPhase;
 import org.elasticsearch.search.fetch.FetchSearchResult;
 import org.elasticsearch.search.fetch.FetchSubPhase;
@@ -32,13 +31,10 @@ import org.elasticsearch.search.internal.InternalSearchHits;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
-/**
- */
-public class InnerHitsFetchSubPhase implements FetchSubPhase {
+public final class InnerHitsFetchSubPhase implements FetchSubPhase {
 
     private final FetchPhase fetchPhase;
 
@@ -46,20 +42,11 @@ public class InnerHitsFetchSubPhase implements FetchSubPhase {
         this.fetchPhase = fetchPhase;
     }
 
-    @Override
-    public Map<String, ? extends SearchParseElement> parseElements() {
-        // SearchParse elements needed because everything is parsed by InnerHitBuilder and eventually put
-        // into the search context.
-        return Collections.emptyMap();
-    }
-
-    @Override
-    public boolean hitExecutionNeeded(SearchContext context) {
-        return context.innerHits() != null && context.innerHits().getInnerHits().size() > 0;
-    }
-
     @Override
     public void hitExecute(SearchContext context, HitContext hitContext) {
+        if ((context.innerHits() != null && context.innerHits().getInnerHits().size() > 0) == false) {
+            return;
+        }
         Map<String, InternalSearchHits> results = new HashMap<>();
         for (Map.Entry<String, InnerHitsContext.BaseInnerHits> entry : context.innerHits().getInnerHits().entrySet()) {
             InnerHitsContext.BaseInnerHits innerHits = entry.getValue();
@@ -92,13 +79,4 @@ public class InnerHitsFetchSubPhase implements FetchSubPhase {
         }
         hitContext.hit().setInnerHits(results);
     }
-
-    @Override
-    public boolean hitsExecutionNeeded(SearchContext context) {
-        return false;
-    }
-
-    @Override
-    public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
-    }
 }

core/src/main/java/org/elasticsearch/search/fetch/matchedqueries/MatchedQueriesFetchSubPhase.java

@@ -18,6 +18,7 @@
  */
 package org.elasticsearch.search.fetch.matchedqueries;
 
+import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.ReaderUtil;
 import org.apache.lucene.search.Query;
@@ -26,7 +27,6 @@ import org.apache.lucene.search.Weight;
 import org.apache.lucene.util.Bits;
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.common.lucene.Lucene;
-import org.elasticsearch.search.SearchParseElement;
 import org.elasticsearch.search.fetch.FetchSubPhase;
 import org.elasticsearch.search.internal.InternalSearchHit;
 import org.elasticsearch.search.internal.SearchContext;
@@ -39,22 +39,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import static java.util.Collections.emptyMap;
-
-/**
- *
- */
-public class MatchedQueriesFetchSubPhase implements FetchSubPhase {
-
-    @Override
-    public Map<String, ? extends SearchParseElement> parseElements() {
-        return emptyMap();
-    }
-
-    @Override
-    public boolean hitsExecutionNeeded(SearchContext context) {
-        return true; // we short-circuit in hitsExecute
-    }
+public final class MatchedQueriesFetchSubPhase implements FetchSubPhase {
 
     @Override
     public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
@@ -82,12 +67,13 @@ public class MatchedQueriesFetchSubPhase implements FetchSubPhase {
                 int docBase = -1;
                 Weight weight = context.searcher().createNormalizedWeight(query, false);
                 Bits matchingDocs = null;
+                final IndexReader indexReader = context.searcher().getIndexReader();
                 for (int i = 0; i < hits.length; ++i) {
                     InternalSearchHit hit = hits[i];
-                    int hitReaderIndex = ReaderUtil.subIndex(hit.docId(), context.searcher().getIndexReader().leaves());
+                    int hitReaderIndex = ReaderUtil.subIndex(hit.docId(), indexReader.leaves());
                     if (readerIndex != hitReaderIndex) {
                         readerIndex = hitReaderIndex;
-                        LeafReaderContext ctx = context.searcher().getIndexReader().leaves().get(readerIndex);
+                        LeafReaderContext ctx = indexReader.leaves().get(readerIndex);
                         docBase = ctx.docBase;
                         // scorers can be costly to create, so reuse them across docs of the same segment
                         Scorer scorer = weight.scorer(ctx);
@@ -99,7 +85,7 @@ public class MatchedQueriesFetchSubPhase implements FetchSubPhase {
                 }
             }
             for (int i = 0; i < hits.length; ++i) {
-                hits[i].matchedQueries(matchedQueries[i].toArray(new String[0]));
+                hits[i].matchedQueries(matchedQueries[i].toArray(new String[matchedQueries[i].size()]));
             }
         } catch (IOException e) {
             throw ExceptionsHelper.convertToElastic(e);
@@ -107,14 +93,4 @@ public class MatchedQueriesFetchSubPhase implements FetchSubPhase {
             SearchContext.current().clearReleasables(Lifetime.COLLECTION);
         }
     }
-
-    @Override
-    public boolean hitExecutionNeeded(SearchContext context) {
-        return false;
-    }
-
-    @Override
-    public void hitExecute(SearchContext context, HitContext hitContext) {
-        // we do everything in hitsExecute
-    }
 }

core/src/main/java/org/elasticsearch/search/fetch/parent/ParentFieldSubFetchPhase.java

@@ -25,10 +25,7 @@ import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.index.mapper.internal.ParentFieldMapper;
 import org.elasticsearch.search.SearchHitField;
-import org.elasticsearch.search.SearchParseElement;
 import org.elasticsearch.search.fetch.FetchSubPhase;
-import org.elasticsearch.search.fetch.innerhits.InnerHitsContext;
-import org.elasticsearch.search.internal.InternalSearchHit;
 import org.elasticsearch.search.internal.InternalSearchHitField;
 import org.elasticsearch.search.internal.SearchContext;
 
@@ -37,17 +34,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
-public class ParentFieldSubFetchPhase implements FetchSubPhase {
-
-    @Override
-    public Map<String, ? extends SearchParseElement> parseElements() {
-        return Collections.emptyMap();
-    }
-
-    @Override
-    public boolean hitExecutionNeeded(SearchContext context) {
-        return true;
-    }
+public final class ParentFieldSubFetchPhase implements FetchSubPhase {
 
     @Override
     public void hitExecute(SearchContext context, HitContext hitContext) {
@@ -65,15 +52,6 @@ public class ParentFieldSubFetchPhase implements FetchSubPhase {
         fields.put(ParentFieldMapper.NAME, new InternalSearchHitField(ParentFieldMapper.NAME, Collections.singletonList(parentId)));
     }
 
-    @Override
-    public boolean hitsExecutionNeeded(SearchContext context) {
-        return false;
-    }
-
-    @Override
-    public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
-    }
-
     public static String getParentId(ParentFieldMapper fieldMapper, LeafReader reader, int docId) {
         try {
             SortedDocValues docValues = reader.getSortedDocValues(fieldMapper.name());

core/src/main/java/org/elasticsearch/search/fetch/script/ScriptFieldsFetchSubPhase.java

@@ -21,7 +21,6 @@ package org.elasticsearch.search.fetch.script;
 import org.elasticsearch.script.LeafSearchScript;
 import org.elasticsearch.search.SearchHitField;
 import org.elasticsearch.search.fetch.FetchSubPhase;
-import org.elasticsearch.search.internal.InternalSearchHit;
 import org.elasticsearch.search.internal.InternalSearchHitField;
 import org.elasticsearch.search.internal.SearchContext;
 
@@ -32,27 +31,13 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 
-/**
- *
- */
-public class ScriptFieldsFetchSubPhase implements FetchSubPhase {
-
-    @Override
-    public boolean hitsExecutionNeeded(SearchContext context) {
-        return false;
-    }
-
-    @Override
-    public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
-    }
-
-    @Override
-    public boolean hitExecutionNeeded(SearchContext context) {
-        return context.hasScriptFields();
-    }
+public final class ScriptFieldsFetchSubPhase implements FetchSubPhase {
 
     @Override
     public void hitExecute(SearchContext context, HitContext hitContext) {
+        if (context.hasScriptFields() == false) {
+            return;
+        }
         for (ScriptFieldsContext.ScriptField scriptField : context.scriptFields().fields()) {
             LeafSearchScript leafScript;
             try {
@@ -62,10 +47,9 @@ public class ScriptFieldsFetchSubPhase implements FetchSubPhase {
             }
             leafScript.setDocument(hitContext.docId());
 
-            Object value;
+            final Object value;
             try {
-                value = leafScript.run();
-                value = leafScript.unwrap(value);
+                value = leafScript.unwrap(leafScript.run());
             } catch (RuntimeException e) {
                 if (scriptField.ignoreException()) {
                     continue;
@@ -74,7 +58,7 @@ public class ScriptFieldsFetchSubPhase implements FetchSubPhase {
             }
 
             if (hitContext.hit().fieldsOrNull() == null) {
-                hitContext.hit().fields(new HashMap<String, SearchHitField>(2));
+                hitContext.hit().fields(new HashMap<>(2));
             }
 
             SearchHitField hitField = hitContext.hit().fields().get(scriptField.name());
@@ -84,7 +68,7 @@ public class ScriptFieldsFetchSubPhase implements FetchSubPhase {
                     values = Collections.emptyList();
                 } else if (value instanceof Collection) {
                     // TODO: use diamond operator once JI-9019884 is fixed
-                    values = new ArrayList<Object>((Collection<?>) value);
+                    values = new ArrayList<>((Collection<?>) value);
                 } else {
                     values = Collections.singletonList(value);
                 }

core/src/main/java/org/elasticsearch/search/fetch/source/FetchSourceSubPhase.java

@@ -23,32 +23,18 @@ import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.search.fetch.FetchSubPhase;
-import org.elasticsearch.search.internal.InternalSearchHit;
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.search.lookup.SourceLookup;
 
 import java.io.IOException;
 
-/**
- */
-public class FetchSourceSubPhase implements FetchSubPhase {
-
-    @Override
-    public boolean hitsExecutionNeeded(SearchContext context) {
-        return false;
-    }
-
-    @Override
-    public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
-    }
-
-    @Override
-    public boolean hitExecutionNeeded(SearchContext context) {
-        return context.sourceRequested();
-    }
+public final class FetchSourceSubPhase implements FetchSubPhase {
 
     @Override
     public void hitExecute(SearchContext context, HitContext hitContext) {
+        if (context.sourceRequested() == false) {
+            return;
+        }
         FetchSourceContext fetchSourceContext = context.fetchSourceContext();
         assert fetchSourceContext.fetchSource();
         if (fetchSourceContext.includes().length == 0 && fetchSourceContext.excludes().length == 0) {

core/src/main/java/org/elasticsearch/search/fetch/version/VersionFetchSubPhase.java

@@ -25,36 +25,21 @@ import org.elasticsearch.common.lucene.uid.Versions;
 import org.elasticsearch.index.mapper.Uid;
 import org.elasticsearch.index.mapper.internal.UidFieldMapper;
 import org.elasticsearch.search.fetch.FetchSubPhase;
-import org.elasticsearch.search.internal.InternalSearchHit;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
 
-/**
- *
- */
-public class VersionFetchSubPhase implements FetchSubPhase {
-
-    @Override
-    public boolean hitsExecutionNeeded(SearchContext context) {
-        return false;
-    }
-
-    @Override
-    public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
-    }
-
-    @Override
-    public boolean hitExecutionNeeded(SearchContext context) {
-        return context.version();
-    }
+public final class VersionFetchSubPhase implements FetchSubPhase {
 
     @Override
     public void hitExecute(SearchContext context, HitContext hitContext) {
+        if (context.version() == false) {
+            return;
+        }
         // it might make sense to cache the TermDocs on a shared fetch context and just skip here)
         // it is going to mean we work on the high level multi reader and not the lower level reader as is
         // the case below...
-        long version;
+        final long version;
         try {
             BytesRef uid = Uid.createUidAsBytes(hitContext.hit().type(), hitContext.hit().id());
             version = Versions.loadVersion(
@@ -64,10 +49,6 @@ public class VersionFetchSubPhase implements FetchSubPhase {
         } catch (IOException e) {
             throw new ElasticsearchException("Could not query index for _version", e);
         }
-
-        if (version < 0) {
-            version = -1;
-        }
-        hitContext.hit().version(version);
+        hitContext.hit().version(version < 0 ? -1 : version);
     }
 }

core/src/main/java/org/elasticsearch/search/highlight/HighlightPhase.java

@@ -21,7 +21,6 @@ package org.elasticsearch.search.highlight;
 
 import org.apache.lucene.search.Query;
 import org.elasticsearch.common.component.AbstractComponent;
-import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.mapper.DocumentMapper;
@@ -31,9 +30,7 @@ import org.elasticsearch.index.mapper.core.StringFieldMapper;
 import org.elasticsearch.index.mapper.core.TextFieldMapper;
 import org.elasticsearch.index.mapper.internal.SourceFieldMapper;
 import org.elasticsearch.search.Highlighters;
-import org.elasticsearch.search.SearchParseElement;
 import org.elasticsearch.search.fetch.FetchSubPhase;
-import org.elasticsearch.search.internal.InternalSearchHit;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.util.Arrays;
@@ -43,45 +40,21 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-/**
- *
- */
 public class HighlightPhase extends AbstractComponent implements FetchSubPhase {
     private static final List<String> STANDARD_HIGHLIGHTERS_BY_PRECEDENCE = Arrays.asList("fvh", "postings", "plain");
 
     private final Highlighters highlighters;
 
-    @Inject
     public HighlightPhase(Settings settings, Highlighters highlighters) {
         super(settings);
         this.highlighters = highlighters;
     }
 
-    /**
-     * highlighters do not have a parse element, they use
-     * {@link HighlightBuilder#fromXContent(org.elasticsearch.index.query.QueryParseContext)} for parsing instead.
-     */
-    @Override
-    public Map<String, ? extends SearchParseElement> parseElements() {
-        return Collections.emptyMap();
-    }
-
-    @Override
-    public boolean hitsExecutionNeeded(SearchContext context) {
-        return false;
-    }
-
-    @Override
-    public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
-    }
-
-    @Override
-    public boolean hitExecutionNeeded(SearchContext context) {
-        return context.highlight() != null;
-    }
-
     @Override
     public void hitExecute(SearchContext context, HitContext hitContext) {
+        if (context.highlight() == null) {
+            return;
+        }
         Map<String, HighlightField> highlightFields = new HashMap<>();
         for (SearchContextHighlight.Field field : context.highlight().fields()) {
             Collection<String> fieldNamesToHighlight;

core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java

@@ -116,7 +116,7 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase {
         }
     }
 
-    public static class TermVectorsFetchSubPhase implements FetchSubPhase {
+    public final static class TermVectorsFetchSubPhase implements FetchSubPhase {
 
         public static final ContextFactory<TermVectorsFetchContext> CONTEXT_FACTORY = new ContextFactory<TermVectorsFetchContext>() {
 
@@ -138,22 +138,11 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase {
             return singletonMap("term_vectors_fetch", new TermVectorsFetchParseElement());
         }
 
-        @Override
-        public boolean hitsExecutionNeeded(SearchContext context) {
-            return false;
-        }
-
-        @Override
-        public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
-        }
-
-        @Override
-        public boolean hitExecutionNeeded(SearchContext context) {
-            return context.getFetchSubPhaseContext(CONTEXT_FACTORY).hitExecutionNeeded();
-        }
-
         @Override
         public void hitExecute(SearchContext context, HitContext hitContext) {
+            if (context.getFetchSubPhaseContext(CONTEXT_FACTORY).hitExecutionNeeded() == false) {
+                return;
+            }
             String field = context.getFetchSubPhaseContext(CONTEXT_FACTORY).getField();
 
             if (hitContext.hit().fieldsOrNull() == null) {

modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhase.java

@@ -32,7 +32,6 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.text.Text;
 import org.elasticsearch.index.query.ParsedQuery;
 import org.elasticsearch.search.Highlighters;
-import org.elasticsearch.search.SearchParseElement;
 import org.elasticsearch.search.fetch.FetchSubPhase;
 import org.elasticsearch.search.highlight.HighlightPhase;
 import org.elasticsearch.search.highlight.SearchContextHighlight;
@@ -43,23 +42,27 @@ import org.elasticsearch.search.internal.SubSearchContext;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
 
-// Highlighting in the case of the percolate query is a bit different, because the PercolateQuery itself doesn't get highlighted,
-// but the source of the PercolateQuery gets highlighted by each hit containing a query.
-public class PercolatorHighlightSubFetchPhase extends HighlightPhase {
+/**
+ * Highlighting in the case of the percolate query is a bit different, because the PercolateQuery itself doesn't get highlighted,
+ * but the source of the PercolateQuery gets highlighted by each hit containing a query.
+ */
+public final class PercolatorHighlightSubFetchPhase extends HighlightPhase {
 
     public PercolatorHighlightSubFetchPhase(Settings settings, Highlighters highlighters) {
         super(settings, highlighters);
     }
 
-    @Override
-    public boolean hitsExecutionNeeded(SearchContext context) {
+
+    boolean hitsExecutionNeeded(SearchContext context) { // for testing
         return context.highlight() != null && locatePercolatorQuery(context.query()) != null;
     }
 
     @Override
     public void hitsExecute(SearchContext context, InternalSearchHit[] hits) {
+        if (hitsExecutionNeeded(context) == false) {
+            return;
+        }
         PercolateQuery percolateQuery = locatePercolatorQuery(context.query());
         if (percolateQuery == null) {
             // shouldn't happen as we checked for the existence of a percolator query in hitsExecutionNeeded(...)
@@ -97,20 +100,6 @@ public class PercolatorHighlightSubFetchPhase extends HighlightPhase {
         }
     }
 
-    @Override
-    public Map<String, ? extends SearchParseElement> parseElements() {
-        return Collections.emptyMap();
-    }
-
-    @Override
-    public boolean hitExecutionNeeded(SearchContext context) {
-        return false;
-    }
-
-    @Override
-    public void hitExecute(SearchContext context, HitContext hitContext) {
-    }
-
     static PercolateQuery locatePercolatorQuery(Query query) {
         if (query instanceof PercolateQuery) {
             return (PercolateQuery) query;

modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhaseTests.java

@@ -53,10 +53,6 @@ public class PercolatorHighlightSubFetchPhaseTests extends ESTestCase {
         Mockito.when(searchContext.query()).thenReturn(new MatchAllDocsQuery());
 
         assertThat(subFetchPhase.hitsExecutionNeeded(searchContext), is(false));
-        IllegalStateException exception = expectThrows(IllegalStateException.class,
-                () -> subFetchPhase.hitsExecute(searchContext, null));
-        assertThat(exception.getMessage(), equalTo("couldn't locate percolator query"));
-
         Mockito.when(searchContext.query()).thenReturn(percolateQuery);
         assertThat(subFetchPhase.hitsExecutionNeeded(searchContext), is(true));
     }