You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ro...@apache.org on 2021/03/30 11:25:13 UTC

[lucene-solr] 02/02: LUCENE-9762: DoubleValuesSource.fromQuery bug (#2365)

This is an automated email from the ASF dual-hosted git repository.

romseygeek pushed a commit to branch branch_8_8
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit bcce43770618bc8ba7dbf33c81f56f80bc073c90
Author: David Smiley <ds...@apache.org>
AuthorDate: Tue Feb 16 22:51:17 2021 -0500

    LUCENE-9762: DoubleValuesSource.fromQuery bug (#2365)
    
    Also used by FunctionScoreQuery.boostByQuery.
    Could throw an exception when the query implements TwoPhaseIterator
    and when the score is requested repeatedly.
    
    Co-authored-by: Chris Hostetter <ho...@apache.org>
    
    (cherry picked from commit 25554180480fdd5722e43ab9356be0f8bab936d0)
---
 lucene/CHANGES.txt                                 |  4 +++
 .../apache/lucene/search/DoubleValuesSource.java   | 14 ++++++--
 .../queries/function/TestFunctionScoreQuery.java   | 38 +++++++++++++++++++---
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 19853e3..2055eb5 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -13,6 +13,10 @@ Bug Fixes
 * LUCENE-9744: NPE on a degenerate query in MinimumShouldMatchIntervalsSource
   $MinimumMatchesIterator.getSubMatches(). (Alan Woodward)
 
+* LUCENE-9762: DoubleValuesSource.fromQuery (also used by FunctionScoreQuery.boostByQuery) could
+  throw an exception when the query implements TwoPhaseIterator and when the score is requested
+  repeatedly. (David Smiley, hossman)
+
 ======================= Lucene 8.8.1 =======================
 
 (No changes)
diff --git a/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java b/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java
index 87e2ecfd..ca82a2c 100644
--- a/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java
+++ b/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java
@@ -493,7 +493,7 @@ public abstract class DoubleValuesSource implements SegmentCacheable {
 
           return new DoubleComparator.DoubleLeafComparator(context) {
             LeafReaderContext ctx;
-            
+
             @Override
             protected NumericDocValues getNumericDocValues(LeafReaderContext context, String field) {
               ctx = context;
@@ -617,6 +617,7 @@ public abstract class DoubleValuesSource implements SegmentCacheable {
       return new DoubleValues() {
         private final TwoPhaseIterator tpi = scorer.twoPhaseIterator();
         private final DocIdSetIterator disi = (tpi == null) ? scorer.iterator() : tpi.approximation();
+        private Boolean tpiMatch = null; // cache tpi.matches()
 
         @Override
         public double doubleValue() throws IOException {
@@ -627,8 +628,17 @@ public abstract class DoubleValuesSource implements SegmentCacheable {
         public boolean advanceExact(int doc) throws IOException {
           if (disi.docID() < doc) {
             disi.advance(doc);
+            tpiMatch = null;
+          }
+          if (disi.docID() == doc) {
+            if (tpi == null) {
+              return true;
+            } else if (tpiMatch == null) {
+              tpiMatch = tpi.matches();
+            }
+            return tpiMatch;
           }
-          return disi.docID() == doc && (tpi == null || tpi.matches());
+          return false;
         }
       };
     }
diff --git a/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreQuery.java b/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreQuery.java
index 6c8ea52..e7a3dda 100644
--- a/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreQuery.java
+++ b/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreQuery.java
@@ -21,13 +21,16 @@ import java.io.IOException;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.lucene.document.Document;
+import org.apache.lucene.document.Field;
 import org.apache.lucene.document.NumericDocValuesField;
+import org.apache.lucene.document.TextField;
 import org.apache.lucene.expressions.Expression;
 import org.apache.lucene.expressions.SimpleBindings;
 import org.apache.lucene.expressions.js.JavascriptCompiler;
 import org.apache.lucene.index.DirectoryReader;
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.IndexWriterConfig;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.BooleanClause;
 import org.apache.lucene.search.BooleanQuery;
@@ -36,6 +39,7 @@ import org.apache.lucene.search.DoubleValuesSource;
 import org.apache.lucene.search.Explanation;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.MatchAllDocsQuery;
+import org.apache.lucene.search.PhraseQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.QueryUtils;
 import org.apache.lucene.search.ScoreMode;
@@ -252,28 +256,52 @@ public class TestFunctionScoreQuery extends FunctionTestSetup {
     assertInnerScoreMode(ScoreMode.COMPLETE_NO_SCORES, ScoreMode.COMPLETE, DoubleValuesSource.fromDoubleField("foo"));
     assertInnerScoreMode(ScoreMode.COMPLETE_NO_SCORES, ScoreMode.COMPLETE_NO_SCORES, DoubleValuesSource.fromDoubleField("foo"));
     assertInnerScoreMode(ScoreMode.COMPLETE_NO_SCORES, ScoreMode.TOP_SCORES, DoubleValuesSource.fromDoubleField("foo"));
-    
+
     // Value Source needs scores
     assertInnerScoreMode(ScoreMode.COMPLETE, ScoreMode.COMPLETE, DoubleValuesSource.SCORES);
     assertInnerScoreMode(ScoreMode.COMPLETE_NO_SCORES, ScoreMode.COMPLETE_NO_SCORES, DoubleValuesSource.SCORES);
     assertInnerScoreMode(ScoreMode.COMPLETE, ScoreMode.TOP_SCORES, DoubleValuesSource.SCORES);
-    
+
   }
-  
+
   private void assertInnerScoreMode(ScoreMode expectedScoreMode, ScoreMode inputScoreMode, DoubleValuesSource valueSource) throws IOException {
     final AtomicReference<ScoreMode> scoreModeInWeight = new AtomicReference<ScoreMode>();
     Query innerQ = new TermQuery(new Term(TEXT_FIELD, "a")) {
-      
+
       @Override
       public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException {
         scoreModeInWeight.set(scoreMode);
         return super.createWeight(searcher, scoreMode, boost);
       }
     };
-    
+
     FunctionScoreQuery fq = new FunctionScoreQuery(innerQ, valueSource);
     fq.createWeight(searcher, inputScoreMode, 1f);
     assertEquals(expectedScoreMode, scoreModeInWeight.get());
   }
 
+  /** The FunctionScoreQuery's Scorer score() is going to be called twice for the same doc. */
+  public void testScoreCalledTwice() throws Exception {
+    try (Directory dir = newDirectory()) {
+      IndexWriterConfig conf = newIndexWriterConfig();
+      IndexWriter indexWriter = new IndexWriter(dir, conf);
+      Document doc = new Document();
+      doc.add(new TextField("ExampleText", "periodic function", Field.Store.NO));
+      doc.add(new TextField("ExampleText", "plot of the original function", Field.Store.NO));
+      indexWriter.addDocument(doc);
+      indexWriter.commit();
+      indexWriter.close();
+
+      try (DirectoryReader reader = DirectoryReader.open(dir)) {
+        Query q = new TermQuery(new Term("ExampleText", "function"));
+
+        q =
+            FunctionScoreQuery.boostByQuery(
+                q, new PhraseQuery(1, "ExampleText", "function", "plot"), 2);
+        q = FunctionScoreQuery.boostByValue(q, DoubleValuesSource.SCORES);
+
+        assertEquals(1, new IndexSearcher(reader).search(q, 10).totalHits.value);
+      }
+    }
+  }
 }