Fix max_score is null when using _score as a primary sort (opensearch-project#18715)

* Fix max_score is null when sorting on score firstly

Signed-off-by: Binlong Gao <[email protected]>

* modify changelog

Signed-off-by: Binlong Gao <[email protected]>

* Fix test failure

Signed-off-by: Binlong Gao <[email protected]>

---------

Signed-off-by: Binlong Gao <[email protected]>

Author: gaobinlong

CHANGELOG.md

@@ -65,6 +65,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix the visit of sub queries for HasParentQuery and HasChildQuery ([#18621](https://github.com/opensearch-project/OpenSearch/pull/18621))
 - Fix the backward compatibility regression with COMPLEMENT for Regexp queries introduced in OpenSearch 3.0 ([#18640](https://github.com/opensearch-project/OpenSearch/pull/18640))
 - Fix Replication lag computation ([#18602](https://github.com/opensearch-project/OpenSearch/pull/18602))
+- Fix max_score is null when sorting on score firstly ([#18715](https://github.com/opensearch-project/OpenSearch/pull/18715))
 - Fixed Staggered merge -  load average replace with AverageTrackers, some Default thresholds modified ([#18666](https://github.com/opensearch-project/OpenSearch/pull/18666))
 
 ### Security

rest-api-spec/src/main/resources/rest-api-spec/test/search/400_max_score.yml

@@ -0,0 +1,89 @@
+setup:
+  - do:
+      indices.create:
+        index: test_1
+        body:
+          mappings:
+            properties:
+              foo:
+                type: keyword
+
+  - do:
+      bulk:
+        refresh: true
+        body:
+          - index:
+              _index: test_1
+          - foo: bar
+          - index:
+              _index: test_1
+          - foo: bar
+
+  - do:
+      indices.refresh:
+        index: [test_1]
+
+---
+teardown:
+  - do:
+      indices.delete:
+        index: test_1
+
+# related issue: https://github.com/opensearch-project/OpenSearch/issues/18714
+---
+"Test max score with sorting on score firstly":
+  - skip:
+      version: " - 3.2.0"
+      reason: Fixed in 3.2.0
+
+  - do:
+      search:
+        index: test_1
+        body:
+          query: { term: { foo: bar} }
+          sort: [{ _score: desc }, { _doc: desc }]
+  - match: { hits.total: 2 }
+  - length: { hits.hits: 2 }
+  - match: { max_score: 1.0 }
+
+  - do:
+      search:
+        index: test_1
+        body:
+          query: { term: { foo: bar} }
+          sort: [{ _score: asc }, { _doc: desc }]
+  - match: { hits.total: 2 }
+  - length: { hits.hits: 2 }
+  - match: { max_score: null }
+
+---
+"Test max score with sorting on score firstly with concurrent segment search enabled":
+  - skip:
+      version: " - 3.2.0"
+      reason: Fixed in 3.2.0
+
+  - do:
+      indices.put_settings:
+        index: test_1
+        body:
+          index.search.concurrent_segment_search.mode: 'all'
+
+  - do:
+      search:
+        index: test_1
+        body:
+          query: { term: { foo: bar} }
+          sort: [{ _score: desc }, { _doc: desc }]
+  - match: { hits.total: 2 }
+  - length: { hits.hits: 2 }
+  - match: { max_score: 1.0 }
+
+  - do:
+      search:
+        index: test_1
+        body:
+          query: { term: { foo: bar} }
+          sort: [{ _score: asc }, { _doc: desc }]
+  - match: { hits.total: 2 }
+  - length: { hits.hits: 2 }
+  - match: { max_score: null }

server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java

@@ -450,6 +450,16 @@ private SimpleTopDocsCollectorContext(
                         return topDocs.scoreDocs[0].score;
                     }
                 };
+            } else if (SortField.FIELD_SCORE.equals(sortAndFormats.sort.getSort()[0])) {
+                maxScoreSupplier = () -> {
+                    TopDocs topDocs = topDocsSupplier.get();
+                    if (topDocs.scoreDocs.length == 0) {
+                        return Float.NaN;
+                    } else {
+                        FieldDoc fieldDoc = (FieldDoc) topDocs.scoreDocs[0];
+                        return (float) fieldDoc.fields[0];
+                    }
+                };
             } else if (trackMaxScore) {
                 maxScoreCollector = new MaxScoreCollector();
                 maxScoreSupplier = maxScoreCollector::getMaxScore;
@@ -595,8 +605,14 @@ TopDocsAndMaxScore newTopDocs(final TopDocs topDocs, final float maxScore, final
                 newTopDocs = new TopDocs(totalHits, scoreDocs);
             }
 
-            if (Float.isNaN(maxScore) && newTopDocs.scoreDocs.length > 0 && sortAndFormats == null) {
-                return new TopDocsAndMaxScore(newTopDocs, newTopDocs.scoreDocs[0].score);
+            if (Float.isNaN(maxScore) && newTopDocs.scoreDocs.length > 0) {
+                float maxScoreFromDoc = maxScore;
+                if (sortAndFormats == null) {
+                    maxScoreFromDoc = newTopDocs.scoreDocs[0].score;
+                } else if (SortField.FIELD_SCORE.equals(sortAndFormats.sort.getSort()[0])) {
+                    maxScoreFromDoc = (float) ((FieldDoc) newTopDocs.scoreDocs[0]).fields[0];
+                }
+                return new TopDocsAndMaxScore(newTopDocs, maxScoreFromDoc);
             } else {
                 return new TopDocsAndMaxScore(newTopDocs, maxScore);
             }

server/src/test/java/org/opensearch/search/query/QueryPhaseTests.java

@@ -994,6 +994,49 @@ public void testMinScore() throws Exception {
         dir.close();
     }
 
+    public void testMaxScoreWithSortOnScoreFirstly() throws Exception {
+        Directory dir = newDirectory();
+        IndexWriterConfig iwc = newIndexWriterConfig();
+        RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc);
+
+        final int numDocs = scaledRandomIntBetween(10, 20);
+        for (int i = 0; i < numDocs; i++) {
+            Document doc = new Document();
+            doc.add(new StringField("foo", "bar", Store.NO));
+            doc.add(new StringField("filter", "f1" + ((i > 0) ? " " + i : ""), Store.NO));
+            w.addDocument(doc);
+        }
+        w.close();
+
+        IndexReader reader = DirectoryReader.open(dir);
+        TestSearchContext context = new TestSearchContext(null, indexShard, newContextSearcher(reader, executor));
+        context.trackScores(false);
+        Sort sort = new Sort(new SortField(null, SortField.Type.SCORE), new SortField(null, SortField.Type.DOC));
+        SortAndFormats sortAndFormats = new SortAndFormats(sort, new DocValueFormat[] { DocValueFormat.RAW, DocValueFormat.RAW });
+        context.sort(sortAndFormats);
+        context.parsedQuery(
+            new ParsedQuery(
+                new BooleanQuery.Builder().add(new TermQuery(new Term("foo", "bar")), Occur.MUST)
+                    .add(new TermQuery(new Term("filter", "f1")), Occur.SHOULD)
+                    .build()
+            )
+        );
+        context.setTask(new SearchShardTask(123L, "", "", "", null, Collections.emptyMap()));
+        context.setSize(1);
+        context.trackTotalHitsUpTo(5);
+
+        QueryPhase.executeInternal(context.withCleanQueryResult(), queryPhaseSearcher);
+        assertFalse(Float.isNaN(context.queryResult().getMaxScore()));
+        assertEquals(1, context.queryResult().topDocs().topDocs.scoreDocs.length);
+        assertThat(
+            ((FieldDoc) context.queryResult().topDocs().topDocs.scoreDocs[0]).fields[0],
+            equalTo(context.queryResult().getMaxScore())
+        );
+
+        reader.close();
+        dir.close();
+    }
+
     public void testMaxScore() throws Exception {
         Directory dir = newDirectory();
         final Sort sort = new Sort(new SortField("filter", SortField.Type.STRING));