From 94e3460305ae652531fbe55a27158490c55c8f0e Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 23 Jan 2017 11:29:21 +0000 Subject: [PATCH] Revert "LUCENE-7628: Scorer.getChildren() returns only matching Scorers" This reverts commit 5bdc492c9ca8f866d9827d83a05fbab4b95f5ce9. --- lucene/CHANGES.txt | 5 -- .../lucene/search/DisjunctionScorer.java | 4 +- .../search/MinShouldMatchSumScorer.java | 16 +++--- .../java/org/apache/lucene/search/Scorer.java | 14 ++--- .../TestBooleanQueryVisitSubscorers.java | 51 ++++--------------- .../lucene/search/TestSubScorerFreqs.java | 19 +++---- .../AssertingSubDocsAtOnceCollector.java | 3 +- .../apache/lucene/search/AssertingScorer.java | 1 - .../org/apache/solr/ltr/LTRScoringQuery.java | 2 +- 9 files changed, 33 insertions(+), 82 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 4d4dd4e5df8..4e90526f4ac 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -77,11 +77,6 @@ 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 8180dc4426b..c53942a71c1 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() throws IOException { + public final Collection getChildren() { ArrayList children = new ArrayList<>(); - for (DisiWrapper scorer = getSubMatches(); scorer != null; scorer = scorer.next) { + for (DisiWrapper scorer : subScorers) { 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 f7604bc4520..c2c419c75e4 100644 --- a/lucene/core/src/java/org/apache/lucene/search/MinShouldMatchSumScorer.java +++ b/lucene/core/src/java/org/apache/lucene/search/MinShouldMatchSumScorer.java @@ -20,6 +20,7 @@ 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; @@ -89,6 +90,7 @@ final class MinShouldMatchSumScorer extends Scorer { final DisiWrapper[] tail; int tailSize; + final Collection childScorers; final long cost; MinShouldMatchSumScorer(Weight weight, Collection scorers, int minShouldMatch) { @@ -113,17 +115,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() 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; + public final Collection getChildren() { + return childScorers; } @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 4387f8d8191..f43432777fe 100644 --- a/lucene/core/src/java/org/apache/lucene/search/Scorer.java +++ b/lucene/core/src/java/org/apache/lucene/search/Scorer.java @@ -77,17 +77,9 @@ public abstract class Scorer { return weight; } - /** - * 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 { + /** Returns child sub-scorers + * @lucene.experimental */ + public Collection getChildren() { 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 092106db28c..38ddcabde7f 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) throws IOException { + private void fillLeaves(Scorer scorer, Set set) { if (scorer.getWeight().getQuery() instanceof TermQuery) { set.add(scorer); } else { @@ -186,40 +186,7 @@ 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 { @@ -236,12 +203,12 @@ public class TestBooleanQueryVisitSubscorers extends LuceneTestCase { for (String summary : collector.getSummaries()) { assertEquals( "ConjunctionScorer\n" + - " MUST ConstantScoreScorer\n" + - " MUST MinShouldMatchSumScorer\n" + - " SHOULD TermScorer body:web\n" + - " SHOULD TermScorer body:crawler\n" + - " SHOULD TermScorer body:nutch", - summary); + " MUST ConstantScoreScorer\n" + + " MUST MinShouldMatchSumScorer\n" + + " SHOULD TermScorer body:nutch\n" + + " SHOULD TermScorer body:crawler\n" + + " SHOULD TermScorer body:web", + summary); } } @@ -294,7 +261,7 @@ public class TestBooleanQueryVisitSubscorers extends LuceneTestCase { }; } - private static void summarizeScorer(final StringBuilder builder, final Scorer scorer, final int indent) throws IOException { + private static void summarizeScorer(final StringBuilder builder, final Scorer scorer, final int indent) { 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 a19dac912e8..121e48dcc74 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestSubScorerFreqs.java @@ -68,6 +68,7 @@ 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) { @@ -78,29 +79,24 @@ public class TestSubScorerFreqs extends LuceneTestCase { super(other); this.relationships = relationships; } - - private Map getSubScorers(Scorer scorer) throws IOException { - Map collected = new HashMap<>(); + + public void setSubScorers(Scorer scorer, String relationship) { for (ChildScorer child : scorer.getChildren()) { if (scorer instanceof AssertingScorer || relationships.contains(child.relationship)) { - collected.put(scorer.getWeight().getQuery(), scorer); + setSubScorers(child.child, child.relationship); } - collected.putAll(getSubScorers(child.child)); } - return collected; + subScorers.put(scorer.getWeight().getQuery(), scorer); } 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(); @@ -113,7 +109,8 @@ public class TestSubScorerFreqs extends LuceneTestCase { @Override public void setScorer(Scorer scorer) throws IOException { super.setScorer(scorer); - this.scorer = scorer; + subScorers.clear(); + setSubScorers(scorer, "TOP"); } }; 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 e545244fd07..793cc412db9 100644 --- a/lucene/facet/src/test/org/apache/lucene/facet/AssertingSubDocsAtOnceCollector.java +++ b/lucene/facet/src/test/org/apache/lucene/facet/AssertingSubDocsAtOnceCollector.java @@ -16,7 +16,6 @@ */ package org.apache.lucene.facet; -import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -33,7 +32,7 @@ class AssertingSubDocsAtOnceCollector extends SimpleCollector { List allScorers; @Override - public void setScorer(Scorer s) throws IOException { + public void setScorer(Scorer s) { // 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 f7149a344c4..1aad1409b4d 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,7 +78,6 @@ 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 37990ea4dd0..b581dd5a859 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() throws IOException { + public Collection getChildren() { return featureTraversalScorer.getChildren(); }