From 632ac1428ec9b0c12bae2d1019a7b0572227fe3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 14 Jun 2021 17:29:53 +0200 Subject: [PATCH] Fix Plain Highlighter ordering for `none` The plain highlighter should order the fragments by order of appearance by default, but sorts them by score internally. This fix changes the sorting comparator in case ordering by score is not selected (the default) and adds testing around this. Closes #58236 --- .../highlight/HighlighterSearchIT.java | 50 ++++++++++++++++++- .../subphase/highlight/PlainHighlighter.java | 5 +- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java index 49a70d94aae33..0c30d02728794 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/highlight/HighlighterSearchIT.java @@ -8,6 +8,7 @@ package org.elasticsearch.search.fetch.subphase.highlight; import com.carrotsearch.randomizedtesting.generators.RandomPicks; + import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.MockAnalyzer; import org.apache.lucene.analysis.MockTokenizer; @@ -693,15 +694,60 @@ public void testPlainHighlighter() throws Exception { .setSource("field1", "this is a test", "field2", "The quick brown fox jumps over the lazy dog").get(); refresh(); - logger.info("--> highlighting and searching on field1"); SearchSourceBuilder source = searchSource() .query(termQuery("field1", "test")) - .highlighter(highlight().field("field1").order("score").preTags("").postTags("")); + .highlighter(highlight().highlighterType("plain").field("field1").order("score").preTags("").postTags("")); SearchResponse searchResponse = client().prepareSearch("test").setSource(source).get(); assertHighlight(searchResponse, 0, "field1", 0, 1, equalTo("this is a test")); + } + + public void testPlainHighlighterOrder() throws Exception { + ensureGreen(); + + client().prepareIndex("test") + .setSource("field1", "The quick brown fox jumps over the lazy brown dog but to no suprise the dog doesn't care").get(); + refresh(); + + { + // fragments should be in order of appearance by default + SearchSourceBuilder source = searchSource().query(matchQuery("field1", "brown dog")) + .highlighter( + highlight().highlighterType("plain").field("field1").preTags("").postTags("").fragmentSize(25) + ); + + SearchResponse searchResponse = client().prepareSearch("test").setSource(source).get(); + + assertHighlight(searchResponse, 0, "field1", 0, 3, equalTo("The quick brown fox")); + assertHighlight(searchResponse, 0, "field1", 1, 3, equalTo(" jumps over the lazy brown dog")); + assertHighlight(searchResponse, 0, "field1", 2, 3, equalTo(" dog doesn't care")); + + // lets be explicit about the order + source = searchSource().query(matchQuery("field1", "brown dog")) + .highlighter( + highlight().highlighterType("plain").field("field1").order("none").preTags("").postTags("").fragmentSize(25) + ); + + searchResponse = client().prepareSearch("test").setSource(source).get(); + + assertHighlight(searchResponse, 0, "field1", 0, 3, equalTo("The quick brown fox")); + assertHighlight(searchResponse, 0, "field1", 1, 3, equalTo(" jumps over the lazy brown dog")); + assertHighlight(searchResponse, 0, "field1", 2, 3, equalTo(" dog doesn't care")); + } + { + // order by score + SearchSourceBuilder source = searchSource().query(matchQuery("field1", "brown dog")) + .highlighter( + highlight().highlighterType("plain").order("score").field("field1").preTags("").postTags("").fragmentSize(25) + ); + SearchResponse searchResponse = client().prepareSearch("test").setSource(source).get(); + + assertHighlight(searchResponse, 0, "field1", 0, 3, equalTo(" jumps over the lazy brown dog")); + assertHighlight(searchResponse, 0, "field1", 1, 3, equalTo("The quick brown fox")); + assertHighlight(searchResponse, 0, "field1", 2, 3, equalTo(" dog doesn't care")); + } } public void testFastVectorHighlighter() throws Exception { diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/PlainHighlighter.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/PlainHighlighter.java index 59755d22489d8..3d4167d9e1e40 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/PlainHighlighter.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/highlight/PlainHighlighter.java @@ -135,8 +135,9 @@ public HighlightField highlight(FieldHighlightContext fieldContext) throws IOExc } } - if (field.fieldOptions().scoreOrdered()) { - CollectionUtil.introSort(fragsList, (o1, o2) -> Math.round(o2.getScore() - o1.getScore())); + // fragments are ordered by score by default since we add them in best + if (field.fieldOptions().scoreOrdered() == false) { + CollectionUtil.introSort(fragsList, (o1, o2) -> o1.getFragNum() - o2.getFragNum()); } String[] fragments; // number_of_fragments is set to 0 but we have a multivalued field