Consistent scores for multi-term SourceConfirmedTestQuery (elastic#10…

…0846) SourceConfirmedTestQuery uses a QueryVisitor to collect terms from its inner query to build its internal SimScorer. It is important to hold these terms in a consistent order so that when scores for each term are summed, the order of summation is the same as it would be for the inner query. This commit changes the call to visit to use a LinkedHashSet to ensure that terms are iterated in the order in which they are collected. Fixes elastic#98712
romseygeek · Oct 16, 2023 · e5674e4 · e5674e4
1 parent 4a9e6e2
commit e5674e4
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 3 deletions.
diff --git a/docs/changelog/100846.yaml b/docs/changelog/100846.yaml
@@ -0,0 +1,6 @@
+pr: 100846
+summary: Consistent scores for multi-term `SourceConfirmedTestQuery`
+area: Search
+type: bug
+issues:
+ - 98712
diff --git a/...-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java b/...-extras/src/main/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQuery.java
@@ -49,7 +49,7 @@
 import java.io.UncheckedIOException;
 import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -215,7 +215,9 @@ public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float bo
             return in.createWeight(searcher, scoreMode, boost);
         }
 
-        final Set<Term> terms = new HashSet<>();
+        // We use a LinkedHashSet here to preserve the ordering of terms to ensure that
+        // later summing of float scores per term is consistent
+        final Set<Term> terms = new LinkedHashSet<>();
         in.visit(QueryVisitor.termCollector(terms));
         if (terms.isEmpty()) {
             throw new IllegalStateException("Query " + in + " doesn't have any term");

diff --git a/...as/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java b/...as/src/test/java/org/elasticsearch/index/mapper/extras/SourceConfirmedTextQueryTests.java
@@ -147,7 +147,6 @@ public void testPhrase() throws Exception {
         }
     }
 
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/98712")
     public void testMultiPhrase() throws Exception {
         try (Directory dir = newDirectory(); IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(Lucene.STANDARD_ANALYZER))) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -147,7 +147,6 @@ public void testPhrase() throws Exception { @@
             }
         }
-        @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/98712")
         public void testMultiPhrase() throws Exception {
             try (Directory dir = newDirectory(); IndexWriter w = new IndexWriter(dir, newIndexWriterConfig(Lucene.STANDARD_ANALYZER))) {
@@ Expand Down @@