LUCENE-8803: Change the way that reverse ordering is implemented.

This addresses some test failures when IndexSearcher is created with an executor and merges hits with TopDocs#merge.
apache · Jul 8, 2019 · ac209b6 · ac209b6
1 parent dd4813d
commit ac209b6
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 5 deletions.
diff --git a/lucene/core/src/java/org/apache/lucene/document/FeatureField.java b/lucene/core/src/java/org/apache/lucene/document/FeatureField.java
@@ -138,7 +138,7 @@ public void setFeatureValue(float featureValue) {
     }
     if (featureValue < Float.MIN_NORMAL) {
       throw new IllegalArgumentException("featureValue must be a positive normal float, got: " +
-          featureValue + "for feature " + fieldsData + " on field " + name +
+          featureValue + " for feature " + fieldsData + " on field " + name +
           " which is less than the minimum positive normal float: " + Float.MIN_NORMAL);
     }
     this.featureValue = featureValue;

diff --git a/lucene/core/src/java/org/apache/lucene/document/FeatureSortField.java b/lucene/core/src/java/org/apache/lucene/document/FeatureSortField.java
@@ -42,7 +42,7 @@ final class FeatureSortField extends SortField {
    * @param featureName The name of the feature to use for the sort value
    */
   public FeatureSortField(String field, String featureName) {
-    super(Objects.requireNonNull(field), SortField.Type.CUSTOM);
+    super(Objects.requireNonNull(field), SortField.Type.CUSTOM, true);
     this.featureName = Objects.requireNonNull(featureName);
   }
 
@@ -128,12 +128,12 @@ private float getValueForDoc(int doc) throws IOException {
 
     @Override
     public int compare(int slot1, int slot2) {
-      return Float.compare(values[slot2], values[slot1]);
+      return Float.compare(values[slot1], values[slot2]);
     }
 
     @Override
     public int compareBottom(int doc) throws IOException {
-      return Float.compare(getValueForDoc(doc), bottom);
+      return Float.compare(bottom, getValueForDoc(doc));
     }
 
     @Override
@@ -158,7 +158,7 @@ public Float value(int slot) {
 
     @Override
     public int compareTop(int doc) throws IOException {
-      return Float.compare(getValueForDoc(doc), topValue);
+      return Float.compare(topValue, getValueForDoc(doc));
     }
   }
 }
diff --git a/lucene/core/src/test/org/apache/lucene/document/TestFeatureSort.java b/lucene/core/src/test/org/apache/lucene/document/TestFeatureSort.java
@@ -21,12 +21,15 @@
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.IndexWriterConfig;
 import org.apache.lucene.index.RandomIndexWriter;
+import org.apache.lucene.search.CheckHits;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
 import org.apache.lucene.search.TopDocs;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.TestUtil;
 
 /*
  * Test for sorting using a feature from a FeatureField.
@@ -216,4 +219,51 @@ public void testFeatureMultipleMissing() throws IOException {
     ir.close();
     dir.close();
   }
+
+  // This duel gives compareBottom and compareTop some coverage
+  public void testDuelFloat() throws IOException {
+    Directory dir = newDirectory();
+    RandomIndexWriter w = new RandomIndexWriter(random(), dir);
+    int numDocs = atLeast(100);
+    for (int d = 0; d < numDocs; ++d) {
+      Document doc = new Document();
+      if (random().nextBoolean()) {
+        float f;
+        do {
+          int freq = TestUtil.nextInt(random(), 1, (1 << 16) - 1);
+          f = FeatureField.decodeFeatureValue(freq);
+        } while (f < Float.MIN_NORMAL);
+        doc.add(new NumericDocValuesField("float", Float.floatToIntBits(f)));
+        doc.add(new FeatureField("feature", "foo", f));
+      }
+      w.addDocument(doc);
+    }
+
+    IndexReader r = w.getReader();
+    w.close();
+    IndexSearcher searcher = newSearcher(r);
+
+    TopDocs topDocs = null;
+    TopDocs featureTopDocs = null;
+    do {
+      if (topDocs == null) {
+        topDocs = searcher.search(new MatchAllDocsQuery(), 10,
+            new Sort(new SortField("float", SortField.Type.FLOAT, true)));
+        featureTopDocs = searcher.search(new MatchAllDocsQuery(), 10,
+            new Sort(FeatureField.newFeatureSort("feature", "foo")));
+      } else {
+        topDocs = searcher.searchAfter(topDocs.scoreDocs[topDocs.scoreDocs.length - 1],
+            new MatchAllDocsQuery(), 10,
+            new Sort(new SortField("float", SortField.Type.FLOAT, true)));
+        featureTopDocs = searcher.searchAfter(featureTopDocs.scoreDocs[featureTopDocs.scoreDocs.length - 1],
+            new MatchAllDocsQuery(), 10,
+            new Sort(FeatureField.newFeatureSort("feature", "foo")));
+      }
+
+      CheckHits.checkEqual(new MatchAllDocsQuery(), topDocs.scoreDocs, featureTopDocs.scoreDocs);
+    } while (topDocs.scoreDocs.length > 0);
+
+    r.close();
+    dir.close();
+  }
 }