Mailing-List: contact commits-help@lucene.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@lucene.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: ab@apache.org
To: commits@lucene.apache.org
Date: Wed, 20 Sep 2017 08:47:01 -0000
Message-Id: <827e4cecaf2b4cb39c0d8c987b095d00@git.apache.org>
In-Reply-To: <b5efad9a0c0e4b95aae33fa2510f61b6@git.apache.org>
References: <b5efad9a0c0e4b95aae33fa2510f61b6@git.apache.org>
Subject: [45/50] lucene-solr:jira/solr-11285: LUCENE-7968: Analyzing
 Suggester's comparator compares incorrectly
archived-at: Wed, 20 Sep 2017 08:46:28 -0000

LUCENE-7968: Analyzing Suggester's comparator compares incorrectly


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/2e5f9a43
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/2e5f9a43
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/2e5f9a43

Branch: refs/heads/jira/solr-11285
Commit: 2e5f9a4369e0f4f664868718ce3ee8fbea43a98b
Parents: ab965d5
Author: Robert Muir <rmuir@apache.org>
Authored: Sun Sep 17 11:37:24 2017 -0400
Committer: Robert Muir <rmuir@apache.org>
Committed: Sun Sep 17 11:37:24 2017 -0400

----------------------------------------------------------------------
 lucene/CHANGES.txt                              |  4 ++
 .../suggest/analyzing/AnalyzingSuggester.java   |  6 +-
 .../analyzing/AnalyzingSuggesterTest.java       | 60 ++++++++++++++++++++
 3 files changed, 68 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/2e5f9a43/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 555c97d..5a95d6c 100644
--- a/lucene/CHANGES.txt
+++ b/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

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/2e5f9a43/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingSuggester.java
----------------------------------------------------------------------
diff --git a/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingSuggester.java b/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingSuggester.java
index 9c6a624..cc11fe1 100644
--- a/lucene/suggest/src/java/org/apache/lucene/search/suggest/analyzing/AnalyzingSuggester.java
+++ b/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;
-        scratchB.length = b.length - scratchB.offset;
+        scratchA.length = readerA.length() - readerA.getPosition();
+        scratchB.length = readerB.length() - readerB.getPosition();
       }
+      assert scratchA.isValid();
+      assert scratchB.isValid();
    
       return scratchA.compareTo(scratchB);
     }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/2e5f9a43/lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/AnalyzingSuggesterTest.java
----------------------------------------------------------------------
diff --git a/lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/AnalyzingSuggesterTest.java b/lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/AnalyzingSuggesterTest.java
index 67ff056..7302d9e 100644
--- a/lucene/suggest/src/test/org/apache/lucene/search/suggest/analyzing/AnalyzingSuggesterTest.java
+++ b/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