LUCENE-7968: Analyzing Suggester's comparator compares incorrectly

lucene/CHANGES.txt

@@ -61,6 +61,10 @@ Bug Fixes
   write.lock for an empty index due to bad permissions/read-only filesystem/etc.
   (Erick Erickson, Shawn Heisey, Robert Muir)
 
+* LUCENE-7968: AnalyzingSuggester would sometimes order suggestions incorrectly,
+  it did not properly break ties on the surface forms when both the weights and
+  the analyzed forms were equal. (Robert Muir)
+
 Build
 
 * SOLR-11181: Switch order of maven artifact publishing procedure: deploy first

lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingSuggester.java

@@ -390,9 +390,11 @@ public class AnalyzingSuggester extends Lookup implements Accountable {
       } else {
         scratchA.offset = readerA.getPosition();
         scratchB.offset = readerB.getPosition();
-        scratchA.length = a.length - scratchA.offset;
+        scratchA.length = readerA.length() - readerA.getPosition();
-        scratchB.length = b.length - scratchB.offset;
+        scratchB.length = readerB.length() - readerB.getPosition();
       }
+      assert scratchA.isValid();
+      assert scratchB.isValid();
    
       return scratchA.compareTo(scratchB);
     }

lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/AnalyzingSuggesterTest.java

@@ -1111,6 +1111,66 @@ public class AnalyzingSuggesterTest extends LuceneTestCase {
     IOUtils.close(a, tempDir);
   }
 
+  /**
+   * Adds 50 random keys, that all analyze to the same thing (dog), with the same cost,
+   * and checks that they come back in surface-form order.
+   */
+  public void testTieBreakOnSurfaceForm() throws Exception {
+    Analyzer a = new Analyzer() {
+      @Override
+      protected TokenStreamComponents createComponents(String fieldName) {
+        Tokenizer tokenizer = new MockTokenizer(MockTokenizer.SIMPLE, true);
+
+        return new TokenStreamComponents(tokenizer) {
+          @Override
+          public TokenStream getTokenStream() {
+            return new CannedTokenStream(new Token[] {
+                token("dog", 1, 1)
+              });
+          }
+          @Override
+          protected void setReader(final Reader reader) {
+          }
+        };
+      }
+    };
+
+    Directory tempDir = getDirectory();
+    AnalyzingSuggester suggester = new AnalyzingSuggester(tempDir, "suggest", a, a, 0, 256, -1, true);
+
+    // make 50 inputs all with the same cost of 1, random strings
+    Input[] inputs = new Input[100];
+    for (int i = 0; i < inputs.length; i++) {
+      inputs[i] = new Input(TestUtil.randomSimpleString(random()), 1);
+    }
+
+    suggester.build(new InputArrayIterator(inputs));
+
+    // Try to save/load:
+    Path tmpDir = createTempDir("AnalyzingSuggesterTest");
+    Path path = tmpDir.resolve("suggester");
+
+    OutputStream os = Files.newOutputStream(path);
+    suggester.store(os);
+    os.close();
+
+    InputStream is = Files.newInputStream(path);
+    suggester.load(is);
+    is.close();
+
+    // now suggest everything, and check that stuff comes back in order
+    List<LookupResult> results = suggester.lookup("", false, 50);
+    assertEquals(50, results.size());
+    for (int i = 1; i < 50; i++) {
+      String previous = results.get(i-1).toString();
+      String current = results.get(i).toString();
+      assertTrue("surface forms out of order: previous=" + previous + ",current=" + current,
+                 current.compareTo(previous) >= 0);
+    }
+
+    IOUtils.close(a, tempDir);
+  }
+
   public void test0ByteKeys() throws Exception {
     final Analyzer a = new Analyzer() {
         @Override