diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 4e90526f4ac..4d4dd4e5df8 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -77,6 +77,11 @@ API Changes * LUCENE-7643: Replaced doc-values queries in lucene/sandbox with factory methods on the *DocValuesField classes. (Adrien Grand) +* LUCENE-7628: Scorer.getChildren() now only returns Scorers that are + positioned on the current document, and can throw an IOException. + AssertingScorer checks that getChildren() is not called on an unpositioned + Scorer. (Alan Woodward, Adrien Grand) + New Features * LUCENE-7623: Add FunctionScoreQuery and FunctionMatchQuery (Alan Woodward, diff --git a/lucene/core/src/java/org/apache/lucene/search/DisjunctionScorer.java b/lucene/core/src/java/org/apache/lucene/search/DisjunctionScorer.java index c53942a71c1..8180dc4426b 100644 --- a/lucene/core/src/java/org/apache/lucene/search/DisjunctionScorer.java +++ b/lucene/core/src/java/org/apache/lucene/search/DisjunctionScorer.java @@ -194,9 +194,9 @@ abstract class DisjunctionScorer extends Scorer { protected abstract float score(DisiWrapper topList) throws IOException; @Override - public final Collection getChildren() { + public final Collection getChildren() throws IOException { ArrayList children = new ArrayList<>(); - for (DisiWrapper scorer : subScorers) { + for (DisiWrapper scorer = getSubMatches(); scorer != null; scorer = scorer.next) { children.add(new ChildScorer(scorer.scorer, "SHOULD")); } return children; diff --git a/lucene/core/src/java/org/apache/lucene/search/MinShouldMatchSumScorer.java b/lucene/core/src/java/org/apache/lucene/search/MinShouldMatchSumScorer.java index c2c419c75e4..f7604bc4520 100644 --- a/lucene/core/src/java/org/apache/lucene/search/MinShouldMatchSumScorer.java +++ b/lucene/core/src/java/org/apache/lucene/search/MinShouldMatchSumScorer.java @@ -20,7 +20,6 @@ package org.apache.lucene.search; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.stream.LongStream; import java.util.stream.StreamSupport; @@ -90,7 +89,6 @@ final class MinShouldMatchSumScorer extends Scorer { final DisiWrapper[] tail; int tailSize; - final Collection childScorers; final long cost; MinShouldMatchSumScorer(Weight weight, Collection scorers, int minShouldMatch) { @@ -115,17 +113,17 @@ final class MinShouldMatchSumScorer extends Scorer { addLead(new DisiWrapper(scorer)); } - List children = new ArrayList<>(); - for (Scorer scorer : scorers) { - children.add(new ChildScorer(scorer, "SHOULD")); - } - this.childScorers = Collections.unmodifiableCollection(children); this.cost = cost(scorers.stream().map(Scorer::iterator).mapToLong(DocIdSetIterator::cost), scorers.size(), minShouldMatch); } @Override - public final Collection getChildren() { - return childScorers; + public final Collection getChildren() throws IOException { + List matchingScorers = new ArrayList<>(); + updateFreq(); + for (DisiWrapper s = lead; s != null; s = s.next) { + matchingScorers.add(new ChildScorer(s.scorer, "SHOULD")); + } + return matchingScorers; } @Override diff --git a/lucene/core/src/java/org/apache/lucene/search/Scorer.java b/lucene/core/src/java/org/apache/lucene/search/Scorer.java index f43432777fe..4387f8d8191 100644 --- a/lucene/core/src/java/org/apache/lucene/search/Scorer.java +++ b/lucene/core/src/java/org/apache/lucene/search/Scorer.java @@ -77,9 +77,17 @@ public abstract class Scorer { return weight; } - /** Returns child sub-scorers - * @lucene.experimental */ - public Collection getChildren() { + /** + * Returns child sub-scorers positioned on the current document + * + * Note that this method should not be called on Scorers passed to {@link LeafCollector#setScorer(Scorer)}, + * as these may be synthetic Scorers produced by {@link BulkScorer} which will throw an Exception. + * + * This method should only be called when the Scorer is positioned + * + * @lucene.experimental + */ + public Collection getChildren() throws IOException { return Collections.emptyList(); } diff --git a/lucene/core/src/test/org/apache/lucene/search/TestBooleanQueryVisitSubscorers.java b/lucene/core/src/test/org/apache/lucene/search/TestBooleanQueryVisitSubscorers.java index 38ddcabde7f..092106db28c 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestBooleanQueryVisitSubscorers.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestBooleanQueryVisitSubscorers.java @@ -169,7 +169,7 @@ public class TestBooleanQueryVisitSubscorers extends LuceneTestCase { }; } - private void fillLeaves(Scorer scorer, Set set) { + private void fillLeaves(Scorer scorer, Set set) throws IOException { if (scorer.getWeight().getQuery() instanceof TermQuery) { set.add(scorer); } else { @@ -186,7 +186,40 @@ public class TestBooleanQueryVisitSubscorers extends LuceneTestCase { public int freq(int doc) throws IOException { return docCounts.get(doc); } - + + } + + public void testDisjunctionMatches() throws IOException { + BooleanQuery.Builder bq1 = new BooleanQuery.Builder(); + bq1.add(new TermQuery(new Term(F1, "lucene")), Occur.SHOULD); + bq1.add(new PhraseQuery(F2, "search", "engine"), Occur.SHOULD); + + Weight w1 = scorerSearcher.createNormalizedWeight(bq1.build(), true); + Scorer s1 = w1.scorer(reader.leaves().get(0)); + assertEquals(0, s1.iterator().nextDoc()); + assertEquals(2, s1.getChildren().size()); + + BooleanQuery.Builder bq2 = new BooleanQuery.Builder(); + bq2.add(new TermQuery(new Term(F1, "lucene")), Occur.SHOULD); + bq2.add(new PhraseQuery(F2, "search", "library"), Occur.SHOULD); + + Weight w2 = scorerSearcher.createNormalizedWeight(bq2.build(), true); + Scorer s2 = w2.scorer(reader.leaves().get(0)); + assertEquals(0, s2.iterator().nextDoc()); + assertEquals(1, s2.getChildren().size()); + } + + public void testMinShouldMatchMatches() throws IOException { + BooleanQuery.Builder bq = new BooleanQuery.Builder(); + bq.add(new TermQuery(new Term(F1, "lucene")), Occur.SHOULD); + bq.add(new TermQuery(new Term(F2, "lucene")), Occur.SHOULD); + bq.add(new PhraseQuery(F2, "search", "library"), Occur.SHOULD); + bq.setMinimumNumberShouldMatch(2); + + Weight w = scorerSearcher.createNormalizedWeight(bq.build(), true); + Scorer s = w.scorer(reader.leaves().get(0)); + assertEquals(0, s.iterator().nextDoc()); + assertEquals(2, s.getChildren().size()); } public void testGetChildrenMinShouldMatchSumScorer() throws IOException { @@ -203,12 +236,12 @@ public class TestBooleanQueryVisitSubscorers extends LuceneTestCase { for (String summary : collector.getSummaries()) { assertEquals( "ConjunctionScorer\n" + - " MUST ConstantScoreScorer\n" + - " MUST MinShouldMatchSumScorer\n" + - " SHOULD TermScorer body:nutch\n" + - " SHOULD TermScorer body:crawler\n" + - " SHOULD TermScorer body:web", - summary); + " MUST ConstantScoreScorer\n" + + " MUST MinShouldMatchSumScorer\n" + + " SHOULD TermScorer body:web\n" + + " SHOULD TermScorer body:crawler\n" + + " SHOULD TermScorer body:nutch", + summary); } } @@ -261,7 +294,7 @@ public class TestBooleanQueryVisitSubscorers extends LuceneTestCase { }; } - private static void summarizeScorer(final StringBuilder builder, final Scorer scorer, final int indent) { + private static void summarizeScorer(final StringBuilder builder, final Scorer scorer, final int indent) throws IOException { builder.append(scorer.getClass().getSimpleName()); if (scorer instanceof TermScorer) { TermQuery termQuery = (TermQuery) scorer.getWeight().getQuery(); diff --git a/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java b/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java index 121e48dcc74..a19dac912e8 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java @@ -68,7 +68,6 @@ public class TestSubScorerFreqs extends LuceneTestCase { private static class CountingCollector extends FilterCollector { public final Map> docCounts = new HashMap<>(); - private final Map subScorers = new HashMap<>(); private final Set relationships; public CountingCollector(Collector other) { @@ -79,24 +78,29 @@ public class TestSubScorerFreqs extends LuceneTestCase { super(other); this.relationships = relationships; } - - public void setSubScorers(Scorer scorer, String relationship) { + + private Map getSubScorers(Scorer scorer) throws IOException { + Map collected = new HashMap<>(); for (ChildScorer child : scorer.getChildren()) { if (scorer instanceof AssertingScorer || relationships.contains(child.relationship)) { - setSubScorers(child.child, child.relationship); + collected.put(scorer.getWeight().getQuery(), scorer); } + collected.putAll(getSubScorers(child.child)); } - subScorers.put(scorer.getWeight().getQuery(), scorer); + return collected; } public LeafCollector getLeafCollector(LeafReaderContext context) throws IOException { final int docBase = context.docBase; return new FilterLeafCollector(super.getLeafCollector(context)) { - + + Scorer scorer; + @Override public void collect(int doc) throws IOException { final Map freqs = new HashMap(); + final Map subScorers = getSubScorers(scorer); for (Map.Entry ent : subScorers.entrySet()) { Scorer value = ent.getValue(); int matchId = value.docID(); @@ -109,8 +113,7 @@ public class TestSubScorerFreqs extends LuceneTestCase { @Override public void setScorer(Scorer scorer) throws IOException { super.setScorer(scorer); - subScorers.clear(); - setSubScorers(scorer, "TOP"); + this.scorer = scorer; } }; diff --git a/lucene/facet/src/test/org/apache/lucene/facet/AssertingSubDocsAtOnceCollector.java b/lucene/facet/src/test/org/apache/lucene/facet/AssertingSubDocsAtOnceCollector.java index 793cc412db9..e545244fd07 100644 --- a/lucene/facet/src/test/org/apache/lucene/facet/AssertingSubDocsAtOnceCollector.java +++ b/lucene/facet/src/test/org/apache/lucene/facet/AssertingSubDocsAtOnceCollector.java @@ -16,6 +16,7 @@ */ package org.apache.lucene.facet; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -32,7 +33,7 @@ class AssertingSubDocsAtOnceCollector extends SimpleCollector { List allScorers; @Override - public void setScorer(Scorer s) { + public void setScorer(Scorer s) throws IOException { // Gathers all scorers, including s and "under": allScorers = new ArrayList<>(); allScorers.add(s); diff --git a/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java b/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java index 1aad1409b4d..f7149a344c4 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java +++ b/lucene/test-framework/src/java/org/apache/lucene/search/AssertingScorer.java @@ -78,6 +78,7 @@ public class AssertingScorer extends Scorer { // collectors (e.g. ToParentBlockJoinCollector) that // need to walk the scorer tree will miss/skip the // Scorer we wrap: + assert iterating(); return Collections.singletonList(new ChildScorer(in, "SHOULD")); } diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRScoringQuery.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRScoringQuery.java index b581dd5a859..37990ea4dd0 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRScoringQuery.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRScoringQuery.java @@ -502,7 +502,7 @@ public class LTRScoringQuery extends Query { } @Override - public Collection getChildren() { + public Collection getChildren() throws IOException { return featureTraversalScorer.getChildren(); }