jpountz commented on code in PR #13319:
URL: https://github.com/apache/lucene/pull/13319#discussion_r1582371473
##########
lucene/core/src/java/org/apache/lucene/document/BinaryRangeFieldRangeQuery.java:
##########
@@ -136,7 +136,18 @@ public float matchCost() {
}
};
- return new ConstantScoreScorer(this, score(), scoreMode, iterator);
+ final var scorer = new ConstantScoreScorer(this, score(), scoreMode,
iterator);
+ return new ScorerSupplier() {
Review Comment:
Since we seem to be doing this in lots of places, we could introduce a
`DefaultScorerSupplier` class that takes a `Scorer` as a ctor argument,
alongside `DefaultBulkScorer`.
##########
lucene/core/src/test/org/apache/lucene/search/TestDisjunctionScoreBlockBoundaryPropagator.java:
##########
@@ -37,8 +37,18 @@ public Explanation explain(LeafReaderContext context, int
doc) throws IOExceptio
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- return null;
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws
IOException {
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ return null;
+ }
+
+ @Override
+ public long cost() {
+ return 0;
+ }
+ };
}
Review Comment:
Let's return a null `ScorerSupplier`?
##########
lucene/core/src/java/org/apache/lucene/document/SortedNumericDocValuesRangeQuery.java:
##########
@@ -101,7 +102,8 @@ public boolean isCacheable(LeafReaderContext ctx) {
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws
IOException {
+ final Weight weight = this;
Review Comment:
Extracting this local variable doesn't seem to help? (maybe it did in a
previous version of the change?)
##########
lucene/core/src/java/org/apache/lucene/search/FilterWeight.java:
##########
@@ -58,11 +58,6 @@ public Explanation explain(LeafReaderContext context, int
doc) throws IOExceptio
return in.explain(context, doc);
}
- @Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- return in.scorer(context);
- }
Review Comment:
This class should delegate scorerSupplier now?
##########
lucene/core/src/test/org/apache/lucene/search/TestBoolean2ScorerSupplier.java:
##########
@@ -42,8 +42,18 @@ public Explanation explain(LeafReaderContext context, int
doc) throws IOExceptio
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- return null;
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws
IOException {
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ return null;
+ }
+
+ @Override
+ public long cost() {
+ return 0;
+ }
+ };
}
Review Comment:
Let's return a null `ScorerSupplier`? Returning null from
`ScorerSupplier#get` is not legal.
##########
lucene/core/src/test/org/apache/lucene/search/TestConjunctionDISI.java:
##########
@@ -98,8 +98,18 @@ public Explanation explain(LeafReaderContext context, int
doc) throws IOExceptio
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- return null;
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws
IOException {
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ return null;
+ }
+
+ @Override
+ public long cost() {
+ return 0;
+ }
+ };
}
Review Comment:
Let's return a null `ScorerSupplier`? Returning null from
`ScorerSupplier#get` is not legal.
##########
lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java:
##########
@@ -1355,9 +1355,23 @@ protected WeightWrapper(Weight in, AtomicBoolean
scorerCalled, AtomicBoolean bul
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- scorerCalled.set(true);
- return in.scorer(context);
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws
IOException {
+ final var scorer = in.scorer(context);
Review Comment:
let's delegate to `scorerSupplier()` not `scorer()`?
##########
lucene/core/src/java/org/apache/lucene/search/Weight.java:
##########
@@ -131,23 +137,7 @@ public final Query getQuery() {
*
* @see #scorer
*/
- public ScorerSupplier scorerSupplier(LeafReaderContext context) throws
IOException {
Review Comment:
Can you update javadocs? E.g. it's not an optional method anymore
##########
lucene/queries/src/java/org/apache/lucene/queries/spans/SpanWeight.java:
##########
@@ -135,16 +135,6 @@ private Similarity.SimScorer buildSimWeight(
public abstract Spans getSpans(LeafReaderContext ctx, Postings
requiredPostings)
throws IOException;
- @Override
- public SpanScorer scorer(LeafReaderContext context) throws IOException {
- final Spans spans = getSpans(context, Postings.POSITIONS);
- if (spans == null) {
- return null;
- }
- final LeafSimScorer docScorer = getSimScorer(context);
- return new SpanScorer(this, spans, docScorer);
- }
-
Review Comment:
I wonder if we need to introduce a `SpanScorerSupplier` class whose `get()`
method returns a `SpanScorer` and make `SpanWeight#scorerSupplier` return this
new type.
##########
lucene/core/src/java/org/apache/lucene/search/Weight.java:
##########
@@ -122,7 +122,13 @@ public final Query getQuery() {
* @return a {@link Scorer} which scores documents in/out-of order.
* @throws IOException if there is a low-level I/O error
*/
- public abstract Scorer scorer(LeafReaderContext context) throws IOException;
+ public final Scorer scorer(LeafReaderContext context) throws IOException {
Review Comment:
Can you update javadocs to mention it's just a helper method that delegates
to scorerSupplier?
##########
lucene/core/src/test/org/apache/lucene/search/TestLRUQueryCache.java:
##########
@@ -1709,12 +1719,22 @@ public int hashCode() {
public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode,
float boost)
throws IOException {
return new ConstantScoreWeight(this, 1) {
-
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- scorerCreatedCount.incrementAndGet();
- return new ConstantScoreScorer(
- this, 1, scoreMode,
DocIdSetIterator.all(context.reader().maxDoc()));
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws
IOException {
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ scorerCreatedCount.incrementAndGet();
+ return new ConstantScoreScorer(
+ weight, 1, scoreMode,
DocIdSetIterator.all(context.reader().maxDoc()));
+ }
+
+ @Override
+ public long cost() {
+ return 0;
Review Comment:
return `context.reader().maxDoc()`?
##########
lucene/core/src/test/org/apache/lucene/search/TestNeedsScores.java:
##########
@@ -131,9 +131,23 @@ public Weight createWeight(IndexSearcher searcher,
ScoreMode scoreMode, float bo
final Weight w = in.createWeight(searcher, scoreMode, boost);
return new FilterWeight(w) {
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- assertEquals("query=" + in, value, scoreMode);
- return w.scorer(context);
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws
IOException {
+ final var scorer = w.scorer(context);
Review Comment:
Can you delegate to `w.scorerSupplier` here as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
