SOLR-6692: When using hl.maxMultiValuedToMatch with hl.preserveMulti, only count matched snippets.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1673216 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -131,7 +131,9 @@ Other Changes
 * SOLR-6692: Default highlighter changes:
   - hl.maxAnalyzedChars now applies cumulatively on a multi-valied field.
   - fragment ranking on a multi-valued field should be more relevant.
-  - Much more extensible.
+  - hl.usePhraseHighlighter is now toggleable on a per-field basis.
+  - Much more extensible (get values from another source; return snippet scores and offsets).
+  - When using hl.maxMultiValuedToMatch with hl.preserveMulti, only count matched snippets.
   (David Smiley)
 
 ==================  5.1.0 ==================

solr/core/src/java/org/apache/solr/highlight/DefaultSolrHighlighter.java

@@ -321,7 +321,7 @@ public class DefaultSolrHighlighter extends SolrHighlighter implements PluginInf
 
   protected SolrFragmentsBuilder getSolrFragmentsBuilder( String fieldName, SolrParams params ){
     String fb = params.getFieldParam( fieldName, HighlightParams.FRAGMENTS_BUILDER );
-    SolrFragmentsBuilder solrFb = fragmentsBuilders.get( fb );
+    SolrFragmentsBuilder solrFb = fragmentsBuilders.get(fb);
     if( solrFb == null ){
       throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "Unknown fragmentsBuilder: " + fb );
     }
@@ -549,7 +549,8 @@ public class DefaultSolrHighlighter extends SolrHighlighter implements PluginInf
           // normally we want a score (must be highlighted), but if preserveMulti then we return a snippet regardless.
           if (bestTextFragment.getScore() > 0 || preserveMulti) {
             frags.add(bestTextFragment);
-            --mvToMatch; // note: limits fragments (for multi-valued fields), not quite the number of values
+            if (bestTextFragment.getScore() > 0)
+              --mvToMatch; // note: limits fragments (for multi-valued fields), not quite the number of values
           }
         }
       } catch (InvalidTokenOffsetsException e) {

solr/core/src/test/org/apache/solr/highlight/HighlighterTest.java

@@ -929,11 +929,11 @@ public class HighlighterTest extends SolrTestCaseJ4 {
         "lower", "gap7 nothing",
         "lower", "gap8 nothing",
         "lower", "gap9 target",
-        "lower", "gap10 target" ));
+        "lower", "gap10 target"));
 
     assertU(commit());
 
-    // First insure we can count all six
+    // First ensure we can count all six
     assertQ("Counting all MV pairs failed",
         req(
             "q", "id:1000",
@@ -946,6 +946,7 @@ public class HighlighterTest extends SolrTestCaseJ4 {
     );
 
     // NOTE: These tests seem repeated, but we're testing for off-by-one errors
+
     // Now we should see exactly 2 by limiting the number of values searched to 4
     assertQ("Off by one by going too far",
         req(
@@ -959,7 +960,6 @@ public class HighlighterTest extends SolrTestCaseJ4 {
         "//lst[@name='highlighting']/lst[@name='1000']/arr[@name='lower' and count(*)=2]"
     );
 
-
     // Does 0 work?
     assertQ("Off by one by going too far",
         req(
@@ -973,7 +973,6 @@ public class HighlighterTest extends SolrTestCaseJ4 {
         "//lst[@name='highlighting']/lst[@name='1000' and count(child::*) = 0]"
     );
 
-
     // Now we should see exactly 2 by limiting the number of values searched to 2
     assertQ("Off by one by not going far enough",
         req(
@@ -987,7 +986,6 @@ public class HighlighterTest extends SolrTestCaseJ4 {
         "//lst[@name='highlighting']/lst[@name='1000']/arr[@name='lower' and count(*)=2]"
     );
 
-
     // Now we should see exactly 1 by limiting the number of values searched to 1
     assertQ("Not counting exactly 1",
         req(
@@ -1001,7 +999,6 @@ public class HighlighterTest extends SolrTestCaseJ4 {
         "//lst[@name='highlighting']/lst[@name='1000']/arr[@name='lower' and count(*)=1]"
     );
 
-
     // Now we should see exactly 4 by limiting the number of values found to 4
     assertQ("Matching 4 should exactly match 4",
         req(
@@ -1015,6 +1012,19 @@ public class HighlighterTest extends SolrTestCaseJ4 {
         "//lst[@name='highlighting']/lst[@name='1000']/arr[@name='lower' and count(*)=4]"
     );
 
+    // But if hl.preserveMulti=true then we should see 6 snippets even though 2 didn't match
+    assertQ("hl.preserveMulti",
+        req(
+            "q", "id:1000",
+            HighlightParams.HIGHLIGHT, "true",
+            HighlightParams.FIELDS, "lower",
+            HighlightParams.Q, "target",
+            HighlightParams.SNIPPETS, "100",
+            HighlightParams.MAX_MULTIVALUED_TO_MATCH, "4",
+            HighlightParams.PRESERVE_MULTI, "true"
+        ),
+        "//lst[@name='highlighting']/lst[@name='1000']/arr[@name='lower' and count(*)=6]"
+    );
 
     // Now we should see exactly 2 by limiting the number of values found to 2
     assertQ("Matching 6 should exactly search them all",
@@ -1029,7 +1039,6 @@ public class HighlighterTest extends SolrTestCaseJ4 {
         "//lst[@name='highlighting']/lst[@name='1000']/arr[@name='lower' and count(*)=6]"
     );
 
-
     // Now we should see exactly 1 by limiting the number of values found to 1
     assertQ("Matching 6 should exactly match them all",
         req(
@@ -1057,7 +1066,6 @@ public class HighlighterTest extends SolrTestCaseJ4 {
     );
 
 
-
     // Should bail at the first parameter matched.
     assertQ("Matching 6 should exactly match them all",
         req(