Bläddra i källkod

FastVectorHighlighter should not cache the field query globally (#25197)

This commit removes the global caching of the field query and replaces it with
a caching per field. Each field can use a different `highlight_query` and the rewriting of
some queries (prefix, automaton, ...) depends on the targeted field so the query used for highlighting
must be unique per field.
There might be a small performance penalty when highlighting multiple fields since the query needs to be rewritten
once per highlighted field with this change.

Fixes #25171
Jim Ferenczi 8 år sedan
förälder
incheckning
68deda6d03

+ 23 - 25
core/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/FastVectorHighlighter.java

@@ -87,29 +87,6 @@ public class FastVectorHighlighter implements Highlighter {
         HighlighterEntry cache = (HighlighterEntry) hitContext.cache().get(CACHE_KEY);
 
         try {
-            FieldQuery fieldQuery;
-            if (field.fieldOptions().requireFieldMatch()) {
-                if (cache.fieldMatchFieldQuery == null) {
-                    /*
-                     * we use top level reader to rewrite the query against all readers,
-                     * with use caching it across hits (and across readers...)
-                     */
-                    cache.fieldMatchFieldQuery = new CustomFieldQuery(highlighterContext.query,
-                        hitContext.topLevelReader(), true, field.fieldOptions().requireFieldMatch());
-                }
-                fieldQuery = cache.fieldMatchFieldQuery;
-            } else {
-                if (cache.noFieldMatchFieldQuery == null) {
-                    /*
-                     * we use top level reader to rewrite the query against all readers,
-                     * with use caching it across hits (and across readers...)
-                     */
-                    cache.noFieldMatchFieldQuery = new CustomFieldQuery(highlighterContext.query,
-                        hitContext.topLevelReader(), true, field.fieldOptions().requireFieldMatch());
-                }
-                fieldQuery = cache.noFieldMatchFieldQuery;
-            }
-
             MapperHighlightEntry entry = cache.mappers.get(mapper);
             if (entry == null) {
                 FragListBuilder fragListBuilder;
@@ -151,6 +128,21 @@ public class FastVectorHighlighter implements Highlighter {
                 }
                 fragmentsBuilder.setDiscreteMultiValueHighlighting(termVectorMultiValue);
                 entry = new MapperHighlightEntry();
+                if (field.fieldOptions().requireFieldMatch()) {
+                    /**
+                     * we use top level reader to rewrite the query against all readers,
+                     * with use caching it across hits (and across readers...)
+                     */
+                    entry.fieldMatchFieldQuery = new CustomFieldQuery(highlighterContext.query,
+                        hitContext.topLevelReader(), true, field.fieldOptions().requireFieldMatch());
+                } else {
+                    /**
+                     * we use top level reader to rewrite the query against all readers,
+                     * with use caching it across hits (and across readers...)
+                     */
+                    entry.noFieldMatchFieldQuery = new CustomFieldQuery(highlighterContext.query,
+                        hitContext.topLevelReader(), true, field.fieldOptions().requireFieldMatch());
+                }
                 entry.fragListBuilder = fragListBuilder;
                 entry.fragmentsBuilder = fragmentsBuilder;
                 if (cache.fvh == null) {
@@ -162,6 +154,12 @@ public class FastVectorHighlighter implements Highlighter {
                 CustomFieldQuery.highlightFilters.set(field.fieldOptions().highlightFilter());
                 cache.mappers.put(mapper, entry);
             }
+            final FieldQuery fieldQuery;
+            if (field.fieldOptions().requireFieldMatch()) {
+                fieldQuery = entry.fieldMatchFieldQuery;
+            } else {
+                fieldQuery = entry.noFieldMatchFieldQuery;
+            }
             cache.fvh.setPhraseLimit(field.fieldOptions().phraseLimit());
 
             String[] fragments;
@@ -249,12 +247,12 @@ public class FastVectorHighlighter implements Highlighter {
     private class MapperHighlightEntry {
         public FragListBuilder fragListBuilder;
         public FragmentsBuilder fragmentsBuilder;
+        public FieldQuery noFieldMatchFieldQuery;
+        public FieldQuery fieldMatchFieldQuery;
     }
 
     private class HighlighterEntry {
         public org.apache.lucene.search.vectorhighlight.FastVectorHighlighter fvh;
-        public FieldQuery noFieldMatchFieldQuery;
-        public FieldQuery fieldMatchFieldQuery;
         public Map<FieldMapper, MapperHighlightEntry> mappers = new HashMap<>();
     }
 }

+ 49 - 0
rest-api-spec/src/main/resources/rest-api-spec/test/search.highlight/20_fvh.yml

@@ -0,0 +1,49 @@
+setup:
+  - do:
+      indices.create:
+          index: test
+          body:
+            mappings:
+              doc:
+                "properties":
+                  "title":
+                     "type": "text"
+                     "term_vector": "with_positions_offsets"
+                  "description":
+                     "type": "text"
+                     "term_vector": "with_positions_offsets"
+  - do:
+      index:
+        index: test
+        type:  doc
+        id:    1
+        body:
+            "title" : "The quick brown fox is brown"
+            "description" : "The quick pink panther is pink"
+  - do:
+      indices.refresh: {}
+
+---
+"Highlight query":
+  - skip:
+      version: " - 5.5.99"
+      reason:  bug fixed in 5.6
+  - do:
+      search:
+        body:
+         highlight:
+          type: fvh
+          fields:
+            description:
+              type: fvh
+              highlight_query:
+                prefix:
+                  description: br
+            title:
+              type: fvh
+              highlight_query:
+                prefix:
+                  title: br
+
+  - match: {hits.hits.0.highlight.title.0: "The quick <em>brown</em> fox is <em>brown</em>"}
+  - is_false: hits.hits.0.highlight.description