Honor topvalue while determining isMissingvalueCompetitive in case bo…

…ttom is not set (#12520)

lucene/CHANGES.txt

@@ -206,6 +206,8 @@ Bug Fixes
   impact on the user, unless you explicitly invoke the convert function of UTF32ToUTF8, and in the extremely rare
   scenario of searching a non-UTF-8 inverted field with Unicode search terms (Tang Donghai).
 
+* LUCENE-12521: Sort After returning in-correct result when missing values are competitive. (Chaitanya Gohel)
+
 Other
 ---------------------
 

lucene/core/src/java/org/apache/lucene/search/comparators/DoubleComparator.java

@@ -96,9 +96,13 @@ public void copy(int slot, int doc) throws IOException {
     }
 
     @Override
-    protected boolean isMissingValueCompetitive() {
-      int result = Double.compare(missingValue, bottom);
-      return reverse ? (result >= 0) : (result <= 0);
+    protected int compareMissingValueWithBottomValue() {
+      return Double.compare(missingValue, bottom);
+    }
+
+    @Override
+    protected int compareMissingValueWithTopValue() {
+      return Double.compare(missingValue, topValue);
     }
 
     @Override

lucene/core/src/java/org/apache/lucene/search/comparators/FloatComparator.java

@@ -96,9 +96,13 @@ public void copy(int slot, int doc) throws IOException {
     }
 
     @Override
-    protected boolean isMissingValueCompetitive() {
-      int result = Float.compare(missingValue, bottom);
-      return reverse ? (result >= 0) : (result <= 0);
+    protected int compareMissingValueWithBottomValue() {
+      return Float.compare(missingValue, bottom);
+    }
+
+    @Override
+    protected int compareMissingValueWithTopValue() {
+      return Float.compare(missingValue, topValue);
     }
 
     @Override

lucene/core/src/java/org/apache/lucene/search/comparators/IntComparator.java

@@ -96,11 +96,13 @@ public void copy(int slot, int doc) throws IOException {
     }
 
     @Override
-    protected boolean isMissingValueCompetitive() {
-      int result = Integer.compare(missingValue, bottom);
-      // in reverse (desc) sort missingValue is competitive when it's greater or equal to bottom,
-      // in asc sort missingValue is competitive when it's smaller or equal to bottom
-      return reverse ? (result >= 0) : (result <= 0);
+    protected int compareMissingValueWithBottomValue() {
+      return Integer.compare(missingValue, bottom);
+    }
+
+    @Override
+    protected int compareMissingValueWithTopValue() {
+      return Integer.compare(missingValue, topValue);
     }
 
     @Override

lucene/core/src/java/org/apache/lucene/search/comparators/LongComparator.java

@@ -96,11 +96,13 @@ public void copy(int slot, int doc) throws IOException {
     }
 
     @Override
-    protected boolean isMissingValueCompetitive() {
-      int result = Long.compare(missingValue, bottom);
-      // in reverse (desc) sort missingValue is competitive when it's greater or equal to bottom,
-      // in asc sort missingValue is competitive when it's smaller or equal to bottom
-      return reverse ? (result >= 0) : (result <= 0);
+    protected int compareMissingValueWithBottomValue() {
+      return Long.compare(missingValue, bottom);
+    }
+
+    @Override
+    protected int compareMissingValueWithTopValue() {
+      return Long.compare(missingValue, topValue);
     }
 
     @Override

lucene/core/src/java/org/apache/lucene/search/comparators/NumericComparator.java

@@ -309,6 +309,26 @@ private void updateSkipInterval(boolean success) {
       }
     }
 
+    private boolean isMissingValueCompetitive() {
+      // if queue is full, always compare with bottom,
+      // if not, check if we can compare with topValue
+      if (queueFull) {
+        int result = compareMissingValueWithBottomValue();
+        // in reverse (desc) sort missingValue is competitive when it's greater or equal to bottom,
+        // in asc sort missingValue is competitive when it's smaller or equal to bottom
+        return reverse ? (result >= 0) : (result <= 0);
+      } else if (topValueSet) {
+        int result = compareMissingValueWithTopValue();
+        // in reverse (desc) sort missingValue is competitive when it's smaller or equal to
+        // topValue,
+        // in asc sort missingValue is competitive when it's greater or equal to topValue
+        return reverse ? (result <= 0) : (result >= 0);
+      } else {
+        // by default competitive
+        return true;
+      }
+    }
+
     @Override
     public DocIdSetIterator competitiveIterator() {
       if (enableSkipping == false) return null;
@@ -337,7 +357,9 @@ public int advance(int target) throws IOException {
       };
     }
 
-    protected abstract boolean isMissingValueCompetitive();
+    protected abstract int compareMissingValueWithTopValue();
+
+    protected abstract int compareMissingValueWithBottomValue();
 
     protected abstract void encodeBottom(byte[] packedValue);
 

lucene/core/src/test/org/apache/lucene/search/TestSortOptimization.java

@@ -239,6 +239,40 @@ public void testSortOptimizationWithMissingValues() throws IOException {
       assertNonCompetitiveHitsAreSkipped(topDocs.totalHits.value, numDocs);
     }
 
+    { // test that optimization is not run when missing value setting of SortField is competitive
+      // with after on asc order
+      final long afterValue = Long.MAX_VALUE;
+      final int afterDocID = 10 + random().nextInt(1000);
+      FieldDoc after = new FieldDoc(afterDocID, Float.NaN, new Long[] {afterValue});
+      final SortField sortField = new SortField("my_field", SortField.Type.LONG);
+      sortField.setMissingValue(Long.MAX_VALUE); // set a competitive missing value
+      final Sort sort = new Sort(sortField);
+      CollectorManager<TopFieldCollector, TopFieldDocs> manager =
+          TopFieldCollector.createSharedManager(sort, numHits, after, totalHitsThreshold);
+      TopDocs topDocs = searcher.search(new MatchAllDocsQuery(), manager);
+      assertEquals(topDocs.scoreDocs.length, numHits);
+      assertEquals(
+          topDocs.totalHits.value,
+          numDocs); // assert that all documents were collected => optimization was not run
+    }
+
+    { // test that optimization is not run when missing value setting of SortField is competitive
+      // with after on desc order
+      final long afterValue = Long.MAX_VALUE;
+      final int afterDocID = 10 + random().nextInt(1000);
+      FieldDoc after = new FieldDoc(afterDocID, Float.NaN, new Long[] {afterValue});
+      final SortField sortField = new SortField("my_field", SortField.Type.LONG, true);
+      sortField.setMissingValue(Long.MAX_VALUE); // set a competitive missing value
+      final Sort sort = new Sort(sortField);
+      CollectorManager<TopFieldCollector, TopFieldDocs> manager =
+          TopFieldCollector.createSharedManager(sort, numHits, after, totalHitsThreshold);
+      TopDocs topDocs = searcher.search(new MatchAllDocsQuery(), manager);
+      assertEquals(topDocs.scoreDocs.length, numHits);
+      assertEquals(
+          topDocs.totalHits.value,
+          numDocs); // assert that all documents were collected => optimization was not run
+    }
+
     reader.close();
     dir.close();
   }