diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index bf8b7353132..3aedf015f25 100644 --- a/solr/CHANGES.txt +++ b/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 ================== diff --git a/solr/core/src/java/org/apache/solr/highlight/DefaultSolrHighlighter.java b/solr/core/src/java/org/apache/solr/highlight/DefaultSolrHighlighter.java index 70ba0e3e7b1..f1c27831900 100644 --- a/solr/core/src/java/org/apache/solr/highlight/DefaultSolrHighlighter.java +++ b/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) { diff --git a/solr/core/src/test/org/apache/solr/highlight/HighlighterTest.java b/solr/core/src/test/org/apache/solr/highlight/HighlighterTest.java index c7552553932..f9e752e4ba3 100644 --- a/solr/core/src/test/org/apache/solr/highlight/HighlighterTest.java +++ b/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(