You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "harshavamsi (via GitHub)" <gi...@apache.org> on 2024/09/10 00:17:31 UTC
[PR] Update QueryUtils to use CollectorManager [lucene]
harshavamsi opened a new pull request, #13748:
URL: https://github.com/apache/lucene/pull/13748
### Description
<!--
If this is your first contribution to Lucene, please make sure you have reviewed the contribution guide.
https://github.com/apache/lucene/blob/main/CONTRIBUTING.md
-->
Relates to https://github.com/apache/lucene/issues/12892
We introduce a new `SimpleCollectorWithLastReader` class to help keep context of LastReader per thread. This way calling `CollectorManager#reduce` can work with an executor.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Update QueryUtils to use CollectorManager [lucene]
Posted by "msfroh (via GitHub)" <gi...@apache.org>.
msfroh commented on code in PR #13748:
URL: https://github.com/apache/lucene/pull/13748#discussion_r1752775181
##########
lucene/test-framework/src/java/org/apache/lucene/tests/search/QueryUtils.java:
##########
@@ -345,175 +348,201 @@ public static void checkSkipTo(final Query q, final IndexSearcher s) throws IOEx
// FUTURE: ensure scorer.doc()==-1
final float maxDiff = 1e-5f;
- final LeafReader[] lastReader = {null};
- s.search(
- q,
- new SimpleCollector() {
- private Scorable sc;
- private Scorer scorer;
- private DocIdSetIterator iterator;
- private int leafPtr;
+ List<LeafReader> lastReaders =
+ s.search(
+ q,
+ new CollectorManager<SimpleCollectorWithLastReader, List<LeafReader>>() {
+ @Override
+ public SimpleCollectorWithLastReader newCollector() {
+ return new SimpleCollectorWithLastReader() {
+ LeafReader lastReader = null;
+ private Scorable sc;
+ private Scorer scorer;
+ private DocIdSetIterator iterator;
+ private int leafPtr;
+
+ @Override
+ public void setScorer(Scorable scorer) {
+ this.sc = scorer;
+ }
- @Override
- public void setScorer(Scorable scorer) {
- this.sc = scorer;
- }
+ @Override
+ public void collect(int doc) throws IOException {
+ float score = sc.score();
+ lastDoc[0] = doc;
+ try {
+ if (scorer == null) {
+ Query rewritten = s.rewrite(q);
+ Weight w = s.createWeight(rewritten, ScoreMode.COMPLETE, 1);
+ LeafReaderContext context = readerContextArray.get(leafPtr);
+ scorer = w.scorer(context);
+ iterator = scorer.iterator();
+ }
+
+ int op = order[(opidx[0]++) % order.length];
+ // System.out.println(op==skip_op ?
+ // "skip("+(sdoc[0]+1)+")":"next()");
+ boolean more =
+ op == skip_op
+ ? iterator.advance(scorer.docID() + 1)
+ != DocIdSetIterator.NO_MORE_DOCS
+ : iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS;
+ int scorerDoc = scorer.docID();
+ float scorerScore = scorer.score();
+ float scorerScore2 = scorer.score();
+ float scoreDiff = Math.abs(score - scorerScore);
+ float scorerDiff = Math.abs(scorerScore2 - scorerScore);
+
+ boolean success = false;
+ try {
+ assertTrue(more);
+ assertEquals("scorerDoc=" + scorerDoc + ",doc=" + doc, scorerDoc, doc);
+ assertTrue(
+ "score=" + score + ", scorerScore=" + scorerScore,
+ scoreDiff <= maxDiff);
+ assertTrue(
+ "scorerScorer=" + scorerScore + ", scorerScore2=" + scorerScore2,
+ scorerDiff <= maxDiff);
+ success = true;
+ } finally {
+ if (!success) {
+ if (LuceneTestCase.VERBOSE) {
+ StringBuilder sbord = new StringBuilder();
+ for (int i = 0; i < order.length; i++) {
+ sbord.append(order[i] == skip_op ? " skip()" : " next()");
+ }
+ System.out.println(
+ "ERROR matching docs:"
+ + "\n\t"
+ + (doc != scorerDoc ? "--> " : "")
+ + "doc="
+ + doc
+ + ", scorerDoc="
+ + scorerDoc
+ + "\n\t"
+ + (!more ? "--> " : "")
+ + "tscorer.more="
+ + more
+ + "\n\t"
+ + (scoreDiff > maxDiff ? "--> " : "")
+ + "scorerScore="
+ + scorerScore
+ + " scoreDiff="
+ + scoreDiff
+ + " maxDiff="
+ + maxDiff
+ + "\n\t"
+ + (scorerDiff > maxDiff ? "--> " : "")
+ + "scorerScore2="
+ + scorerScore2
+ + " scorerDiff="
+ + scorerDiff
+ + "\n\thitCollector.doc="
+ + doc
+ + " score="
+ + score
+ + "\n\t Scorer="
+ + scorer
+ + "\n\t Query="
+ + q
+ + " "
+ + q.getClass().getName()
+ + "\n\t Searcher="
+ + s
+ + "\n\t Order="
+ + sbord
+ + "\n\t Op="
+ + (op == skip_op ? " skip()" : " next()"));
+ }
+ }
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
- @Override
- public void collect(int doc) throws IOException {
- float score = sc.score();
- lastDoc[0] = doc;
- try {
- if (scorer == null) {
- Query rewritten = s.rewrite(q);
- Weight w = s.createWeight(rewritten, ScoreMode.COMPLETE, 1);
- LeafReaderContext context = readerContextArray.get(leafPtr);
- scorer = w.scorer(context);
- iterator = scorer.iterator();
- }
+ @Override
+ public ScoreMode scoreMode() {
+ return ScoreMode.COMPLETE;
+ }
- int op = order[(opidx[0]++) % order.length];
- // System.out.println(op==skip_op ?
- // "skip("+(sdoc[0]+1)+")":"next()");
- boolean more =
- op == skip_op
- ? iterator.advance(scorer.docID() + 1) != DocIdSetIterator.NO_MORE_DOCS
- : iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS;
- int scorerDoc = scorer.docID();
- float scorerScore = scorer.score();
- float scorerScore2 = scorer.score();
- float scoreDiff = Math.abs(score - scorerScore);
- float scorerDiff = Math.abs(scorerScore2 - scorerScore);
-
- boolean success = false;
- try {
- assertTrue(more);
- assertEquals("scorerDoc=" + scorerDoc + ",doc=" + doc, scorerDoc, doc);
- assertTrue(
- "score=" + score + ", scorerScore=" + scorerScore, scoreDiff <= maxDiff);
- assertTrue(
- "scorerScorer=" + scorerScore + ", scorerScore2=" + scorerScore2,
- scorerDiff <= maxDiff);
- success = true;
- } finally {
- if (!success) {
- if (LuceneTestCase.VERBOSE) {
- StringBuilder sbord = new StringBuilder();
- for (int i = 0; i < order.length; i++) {
- sbord.append(order[i] == skip_op ? " skip()" : " next()");
+ @Override
+ protected void doSetNextReader(LeafReaderContext context) throws IOException {
+ // confirm that skipping beyond the last doc, on the
+ // previous reader, hits NO_MORE_DOCS
+ if (lastReader != null) {
+ final LeafReader previousReader = lastReader;
+ IndexSearcher indexSearcher =
+ LuceneTestCase.newSearcher(previousReader, false);
+ indexSearcher.setSimilarity(s.getSimilarity());
+ Query rewritten = indexSearcher.rewrite(q);
+ Weight w = indexSearcher.createWeight(rewritten, ScoreMode.COMPLETE, 1);
+ LeafReaderContext ctx =
+ (LeafReaderContext) indexSearcher.getTopReaderContext();
+ Scorer scorer = w.scorer(ctx);
+ if (scorer != null) {
+ DocIdSetIterator iterator = scorer.iterator();
+ boolean more = false;
+ final Bits liveDocs = context.reader().getLiveDocs();
+ for (int d = iterator.advance(lastDoc[0] + 1);
+ d != DocIdSetIterator.NO_MORE_DOCS;
+ d = iterator.nextDoc()) {
+ if (liveDocs == null || liveDocs.get(d)) {
+ more = true;
+ break;
+ }
+ }
+ Assert.assertFalse(
+ "query's last doc was "
+ + lastDoc[0]
+ + " but advance("
+ + (lastDoc[0] + 1)
+ + ") got to "
+ + scorer.docID(),
+ more);
+ }
+ leafPtr++;
}
- System.out.println(
- "ERROR matching docs:"
- + "\n\t"
- + (doc != scorerDoc ? "--> " : "")
- + "doc="
- + doc
- + ", scorerDoc="
- + scorerDoc
- + "\n\t"
- + (!more ? "--> " : "")
- + "tscorer.more="
- + more
- + "\n\t"
- + (scoreDiff > maxDiff ? "--> " : "")
- + "scorerScore="
- + scorerScore
- + " scoreDiff="
- + scoreDiff
- + " maxDiff="
- + maxDiff
- + "\n\t"
- + (scorerDiff > maxDiff ? "--> " : "")
- + "scorerScore2="
- + scorerScore2
- + " scorerDiff="
- + scorerDiff
- + "\n\thitCollector.doc="
- + doc
- + " score="
- + score
- + "\n\t Scorer="
- + scorer
- + "\n\t Query="
- + q
- + " "
- + q.getClass().getName()
- + "\n\t Searcher="
- + s
- + "\n\t Order="
- + sbord
- + "\n\t Op="
- + (op == skip_op ? " skip()" : " next()"));
+ lastReader = context.reader();
+ assert readerContextArray.get(leafPtr).reader() == context.reader();
Review Comment:
This assert is failing.
I assume it's because each slice gets a subset of segments, but the old logic assumed that the LeafReaders would be passed to the collector in the same order that they're in the top-level reader.
In a concurrent search world, I don't think this assertion adds value. As long as all the segments get collected, I don't think the order really matters.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Update QueryUtils to use CollectorManager [lucene]
Posted by "msfroh (via GitHub)" <gi...@apache.org>.
msfroh commented on PR #13748:
URL: https://github.com/apache/lucene/pull/13748#issuecomment-2339574918
I had another thought while walking home. Both of the anonymous CollectorManagers are the same (but produce slightly different collectors). What if you split it out into an inner class that takes a `Supplier<SimpleCollectorWithLeafReader>` as a constructor arg?
That way the two checks can each have their anonymous implementations, but the anonymous nesting won't be as deep.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Update QueryUtils to use CollectorManager [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13748:
URL: https://github.com/apache/lucene/pull/13748#issuecomment-2375493347
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Update QueryUtils to use CollectorManager [lucene]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13748:
URL: https://github.com/apache/lucene/pull/13748#issuecomment-3408724084
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Update QueryUtils to use CollectorManager [lucene]
Posted by "msfroh (via GitHub)" <gi...@apache.org>.
msfroh commented on code in PR #13748:
URL: https://github.com/apache/lucene/pull/13748#discussion_r1751112687
##########
lucene/test-framework/src/java/org/apache/lucene/tests/search/QueryUtils.java:
##########
@@ -345,175 +348,199 @@ public static void checkSkipTo(final Query q, final IndexSearcher s) throws IOEx
// FUTURE: ensure scorer.doc()==-1
final float maxDiff = 1e-5f;
- final LeafReader[] lastReader = {null};
- s.search(
- q,
- new SimpleCollector() {
- private Scorable sc;
- private Scorer scorer;
- private DocIdSetIterator iterator;
- private int leafPtr;
+ List<LeafReader> lastReaders =
+ s.search(
+ q,
+ new CollectorManager<SimpleCollectorWithLastReader, List<LeafReader>>() {
+ @Override
+ public SimpleCollectorWithLastReader newCollector() {
+ return new SimpleCollectorWithLastReader() {
+ LeafReader lastReader = null;
+ private Scorable sc;
+ private Scorer scorer;
+ private DocIdSetIterator iterator;
+ private int leafPtr;
+
+ @Override
+ public void setScorer(Scorable scorer) {
+ this.sc = scorer;
+ }
- @Override
- public void setScorer(Scorable scorer) {
- this.sc = scorer;
- }
+ @Override
+ public void collect(int doc) throws IOException {
+ float score = sc.score();
+ lastDoc[0] = doc;
+ try {
+ if (scorer == null) {
+ Query rewritten = s.rewrite(q);
+ Weight w = s.createWeight(rewritten, ScoreMode.COMPLETE, 1);
+ LeafReaderContext context = readerContextArray.get(leafPtr);
+ scorer = w.scorer(context);
+ iterator = scorer.iterator();
+ }
+
+ int op = order[(opidx[0]++) % order.length];
+ // System.out.println(op==skip_op ?
+ // "skip("+(sdoc[0]+1)+")":"next()");
+ boolean more =
+ op == skip_op
+ ? iterator.advance(scorer.docID() + 1)
+ != DocIdSetIterator.NO_MORE_DOCS
+ : iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS;
+ int scorerDoc = scorer.docID();
+ float scorerScore = scorer.score();
+ float scorerScore2 = scorer.score();
+ float scoreDiff = Math.abs(score - scorerScore);
+ float scorerDiff = Math.abs(scorerScore2 - scorerScore);
+
+ boolean success = false;
+ try {
+ assertTrue(more);
+ assertEquals("scorerDoc=" + scorerDoc + ",doc=" + doc, scorerDoc, doc);
+ assertTrue(
+ "score=" + score + ", scorerScore=" + scorerScore,
+ scoreDiff <= maxDiff);
+ assertTrue(
+ "scorerScorer=" + scorerScore + ", scorerScore2=" + scorerScore2,
+ scorerDiff <= maxDiff);
+ success = true;
+ } finally {
+ if (!success) {
+ if (LuceneTestCase.VERBOSE) {
+ StringBuilder sbord = new StringBuilder();
+ for (int i = 0; i < order.length; i++) {
+ sbord.append(order[i] == skip_op ? " skip()" : " next()");
+ }
+ System.out.println(
+ "ERROR matching docs:"
+ + "\n\t"
+ + (doc != scorerDoc ? "--> " : "")
+ + "doc="
+ + doc
+ + ", scorerDoc="
+ + scorerDoc
+ + "\n\t"
+ + (!more ? "--> " : "")
+ + "tscorer.more="
+ + more
+ + "\n\t"
+ + (scoreDiff > maxDiff ? "--> " : "")
+ + "scorerScore="
+ + scorerScore
+ + " scoreDiff="
+ + scoreDiff
+ + " maxDiff="
+ + maxDiff
+ + "\n\t"
+ + (scorerDiff > maxDiff ? "--> " : "")
+ + "scorerScore2="
+ + scorerScore2
+ + " scorerDiff="
+ + scorerDiff
+ + "\n\thitCollector.doc="
+ + doc
+ + " score="
+ + score
+ + "\n\t Scorer="
+ + scorer
+ + "\n\t Query="
+ + q
+ + " "
+ + q.getClass().getName()
+ + "\n\t Searcher="
+ + s
+ + "\n\t Order="
+ + sbord
+ + "\n\t Op="
+ + (op == skip_op ? " skip()" : " next()"));
+ }
+ }
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
- @Override
- public void collect(int doc) throws IOException {
- float score = sc.score();
- lastDoc[0] = doc;
- try {
- if (scorer == null) {
- Query rewritten = s.rewrite(q);
- Weight w = s.createWeight(rewritten, ScoreMode.COMPLETE, 1);
- LeafReaderContext context = readerContextArray.get(leafPtr);
- scorer = w.scorer(context);
- iterator = scorer.iterator();
- }
+ @Override
+ public ScoreMode scoreMode() {
+ return ScoreMode.COMPLETE;
+ }
- int op = order[(opidx[0]++) % order.length];
- // System.out.println(op==skip_op ?
- // "skip("+(sdoc[0]+1)+")":"next()");
- boolean more =
- op == skip_op
- ? iterator.advance(scorer.docID() + 1) != DocIdSetIterator.NO_MORE_DOCS
- : iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS;
- int scorerDoc = scorer.docID();
- float scorerScore = scorer.score();
- float scorerScore2 = scorer.score();
- float scoreDiff = Math.abs(score - scorerScore);
- float scorerDiff = Math.abs(scorerScore2 - scorerScore);
-
- boolean success = false;
- try {
- assertTrue(more);
- assertEquals("scorerDoc=" + scorerDoc + ",doc=" + doc, scorerDoc, doc);
- assertTrue(
- "score=" + score + ", scorerScore=" + scorerScore, scoreDiff <= maxDiff);
- assertTrue(
- "scorerScorer=" + scorerScore + ", scorerScore2=" + scorerScore2,
- scorerDiff <= maxDiff);
- success = true;
- } finally {
- if (!success) {
- if (LuceneTestCase.VERBOSE) {
- StringBuilder sbord = new StringBuilder();
- for (int i = 0; i < order.length; i++) {
- sbord.append(order[i] == skip_op ? " skip()" : " next()");
+ @Override
+ protected void doSetNextReader(LeafReaderContext context) throws IOException {
+ // confirm that skipping beyond the last doc, on the
+ // previous reader, hits NO_MORE_DOCS
+ if (lastReader != null) {
+ final LeafReader previousReader = lastReader;
+ IndexSearcher indexSearcher =
+ LuceneTestCase.newSearcher(previousReader, false);
+ indexSearcher.setSimilarity(s.getSimilarity());
+ Query rewritten = indexSearcher.rewrite(q);
+ Weight w = indexSearcher.createWeight(rewritten, ScoreMode.COMPLETE, 1);
+ LeafReaderContext ctx =
+ (LeafReaderContext) indexSearcher.getTopReaderContext();
+ Scorer scorer = w.scorer(ctx);
+ if (scorer != null) {
+ DocIdSetIterator iterator = scorer.iterator();
+ boolean more = false;
+ final Bits liveDocs = context.reader().getLiveDocs();
+ for (int d = iterator.advance(lastDoc[0] + 1);
+ d != DocIdSetIterator.NO_MORE_DOCS;
+ d = iterator.nextDoc()) {
+ if (liveDocs == null || liveDocs.get(d)) {
+ more = true;
+ break;
+ }
+ }
+ Assert.assertFalse(
+ "query's last doc was "
+ + lastDoc[0]
+ + " but advance("
+ + (lastDoc[0] + 1)
+ + ") got to "
+ + scorer.docID(),
+ more);
+ }
+ leafPtr++;
}
- System.out.println(
- "ERROR matching docs:"
- + "\n\t"
- + (doc != scorerDoc ? "--> " : "")
- + "doc="
- + doc
- + ", scorerDoc="
- + scorerDoc
- + "\n\t"
- + (!more ? "--> " : "")
- + "tscorer.more="
- + more
- + "\n\t"
- + (scoreDiff > maxDiff ? "--> " : "")
- + "scorerScore="
- + scorerScore
- + " scoreDiff="
- + scoreDiff
- + " maxDiff="
- + maxDiff
- + "\n\t"
- + (scorerDiff > maxDiff ? "--> " : "")
- + "scorerScore2="
- + scorerScore2
- + " scorerDiff="
- + scorerDiff
- + "\n\thitCollector.doc="
- + doc
- + " score="
- + score
- + "\n\t Scorer="
- + scorer
- + "\n\t Query="
- + q
- + " "
- + q.getClass().getName()
- + "\n\t Searcher="
- + s
- + "\n\t Order="
- + sbord
- + "\n\t Op="
- + (op == skip_op ? " skip()" : " next()"));
+ lastReader = context.reader();
+ assert readerContextArray.get(leafPtr).reader() == context.reader();
+ this.scorer = null;
+ lastDoc[0] = -1;
}
- }
- }
- } catch (IOException e) {
- throw new RuntimeException(e);
- }
- }
- @Override
- public ScoreMode scoreMode() {
- return ScoreMode.COMPLETE;
- }
-
- @Override
- protected void doSetNextReader(LeafReaderContext context) throws IOException {
- // confirm that skipping beyond the last doc, on the
- // previous reader, hits NO_MORE_DOCS
- if (lastReader[0] != null) {
- final LeafReader previousReader = lastReader[0];
- IndexSearcher indexSearcher = LuceneTestCase.newSearcher(previousReader, false);
- indexSearcher.setSimilarity(s.getSimilarity());
- Query rewritten = indexSearcher.rewrite(q);
- Weight w = indexSearcher.createWeight(rewritten, ScoreMode.COMPLETE, 1);
- LeafReaderContext ctx = (LeafReaderContext) indexSearcher.getTopReaderContext();
- Scorer scorer = w.scorer(ctx);
- if (scorer != null) {
- DocIdSetIterator iterator = scorer.iterator();
- boolean more = false;
- final Bits liveDocs = context.reader().getLiveDocs();
- for (int d = iterator.advance(lastDoc[0] + 1);
- d != DocIdSetIterator.NO_MORE_DOCS;
- d = iterator.nextDoc()) {
- if (liveDocs == null || liveDocs.get(d)) {
- more = true;
- break;
+ @Override
+ public LeafReader getLastReader() {
+ return null;
}
Review Comment:
You'll need to return `lastReader`.
##########
lucene/test-framework/src/java/org/apache/lucene/tests/search/QueryUtils.java:
##########
@@ -540,117 +567,148 @@ public static void checkFirstSkipTo(final Query q, final IndexSearcher s) throws
// System.out.println("checkFirstSkipTo: "+q);
final float maxDiff = 1e-3f;
final int[] lastDoc = {-1};
- final LeafReader[] lastReader = {null};
final List<LeafReaderContext> context = s.getTopReaderContext().leaves();
Query rewritten = s.rewrite(q);
- s.search(
- q,
- new SimpleCollector() {
- private final Weight w = s.createWeight(rewritten, ScoreMode.COMPLETE, 1);
- private Scorable scorer;
- private int leafPtr;
- private long intervalTimes32 = 1 * 32;
+ List<LeafReader> lastReaders =
+ s.search(
+ q,
+ new CollectorManager<SimpleCollectorWithLastReader, List<LeafReader>>() {
+ @Override
+ public SimpleCollectorWithLastReader newCollector() throws IOException {
+ return new SimpleCollectorWithLastReader() {
+ LeafReader lastReader = null;
+ private final Weight w = s.createWeight(rewritten, ScoreMode.COMPLETE, 1);
+ private Scorable scorer;
+ private int leafPtr;
+ private long intervalTimes32 = 1 * 32;
- @Override
- public void setScorer(Scorable scorer) {
- this.scorer = scorer;
- }
+ @Override
+ public void setScorer(Scorable scorer) {
+ this.scorer = scorer;
+ }
- @Override
- public void collect(int doc) throws IOException {
- float score = scorer.score();
- try {
- // The intervalTimes32 trick helps contain the runtime of this check: first we check
- // every single doc in the interval, then after 32 docs we check every 2 docs, etc.
- for (int i = lastDoc[0] + 1; i <= doc; i += intervalTimes32++ / 1024) {
- ScorerSupplier supplier = w.scorerSupplier(context.get(leafPtr));
- Scorer scorer = supplier.get(1L); // only checking one doc, so leadCost = 1
- assertTrue(
- "query collected " + doc + " but advance(" + i + ") says no more docs!",
- scorer.iterator().advance(i) != DocIdSetIterator.NO_MORE_DOCS);
- assertEquals(
- "query collected " + doc + " but advance(" + i + ") got to " + scorer.docID(),
- doc,
- scorer.docID());
- float advanceScore = scorer.score();
- assertEquals(
- "unstable advance(" + i + ") score!", advanceScore, scorer.score(), maxDiff);
- assertEquals(
- "query assigned doc "
- + doc
- + " a score of <"
- + score
- + "> but advance("
- + i
- + ") has <"
- + advanceScore
- + ">!",
- score,
- advanceScore,
- maxDiff);
- }
- lastDoc[0] = doc;
- } catch (IOException e) {
- throw new RuntimeException(e);
- }
- }
+ @Override
+ public void collect(int doc) throws IOException {
+ float score = scorer.score();
+ try {
+ // The intervalTimes32 trick helps contain the runtime of this check: first we
+ // check
+ // every single doc in the interval, then after 32 docs we check every 2 docs,
+ // etc.
+ for (int i = lastDoc[0] + 1; i <= doc; i += intervalTimes32++ / 1024) {
+ ScorerSupplier supplier = w.scorerSupplier(context.get(leafPtr));
+ Scorer scorer = supplier.get(1L); // only checking one doc, so leadCost = 1
+ assertTrue(
+ "query collected " + doc + " but advance(" + i + ") says no more docs!",
+ scorer.iterator().advance(i) != DocIdSetIterator.NO_MORE_DOCS);
+ assertEquals(
+ "query collected "
+ + doc
+ + " but advance("
+ + i
+ + ") got to "
+ + scorer.docID(),
+ doc,
+ scorer.docID());
+ float advanceScore = scorer.score();
+ assertEquals(
+ "unstable advance(" + i + ") score!",
+ advanceScore,
+ scorer.score(),
+ maxDiff);
+ assertEquals(
+ "query assigned doc "
+ + doc
+ + " a score of <"
+ + score
+ + "> but advance("
+ + i
+ + ") has <"
+ + advanceScore
+ + ">!",
+ score,
+ advanceScore,
+ maxDiff);
+ }
+ lastDoc[0] = doc;
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
- @Override
- public ScoreMode scoreMode() {
- return ScoreMode.COMPLETE;
- }
+ @Override
+ public ScoreMode scoreMode() {
+ return ScoreMode.COMPLETE;
+ }
- @Override
- protected void doSetNextReader(LeafReaderContext context) throws IOException {
- // confirm that skipping beyond the last doc, on the
- // previous reader, hits NO_MORE_DOCS
- if (lastReader[0] != null) {
- final LeafReader previousReader = lastReader[0];
- IndexSearcher indexSearcher = LuceneTestCase.newSearcher(previousReader, false);
- indexSearcher.setSimilarity(s.getSimilarity());
- Weight w = indexSearcher.createWeight(rewritten, ScoreMode.COMPLETE, 1);
- Scorer scorer = w.scorer((LeafReaderContext) indexSearcher.getTopReaderContext());
- if (scorer != null) {
- DocIdSetIterator iterator = scorer.iterator();
- boolean more = false;
- final Bits liveDocs = context.reader().getLiveDocs();
- for (int d = iterator.advance(lastDoc[0] + 1);
- d != DocIdSetIterator.NO_MORE_DOCS;
- d = iterator.nextDoc()) {
- if (liveDocs == null || liveDocs.get(d)) {
- more = true;
- break;
+ @Override
+ protected void doSetNextReader(LeafReaderContext context) throws IOException {
+ // confirm that skipping beyond the last doc, on the
+ // previous reader, hits NO_MORE_DOCS
+ if (lastReader != null) {
+ final LeafReader previousReader = lastReader;
+ IndexSearcher indexSearcher =
+ LuceneTestCase.newSearcher(previousReader, false);
+ indexSearcher.setSimilarity(s.getSimilarity());
+ Weight w = indexSearcher.createWeight(rewritten, ScoreMode.COMPLETE, 1);
+ Scorer scorer =
+ w.scorer((LeafReaderContext) indexSearcher.getTopReaderContext());
+ if (scorer != null) {
+ DocIdSetIterator iterator = scorer.iterator();
+ boolean more = false;
+ final Bits liveDocs = context.reader().getLiveDocs();
+ for (int d = iterator.advance(lastDoc[0] + 1);
+ d != DocIdSetIterator.NO_MORE_DOCS;
+ d = iterator.nextDoc()) {
+ if (liveDocs == null || liveDocs.get(d)) {
+ more = true;
+ break;
+ }
+ }
+ assertFalse(
+ "query's last doc was "
+ + lastDoc[0]
+ + " but advance("
+ + (lastDoc[0] + 1)
+ + ") got to "
+ + scorer.docID(),
+ more);
+ }
+ leafPtr++;
+ }
+
+ lastReader = context.reader();
+ lastDoc[0] = -1;
}
- }
- assertFalse(
- "query's last doc was "
- + lastDoc[0]
- + " but advance("
- + (lastDoc[0] + 1)
- + ") got to "
- + scorer.docID(),
- more);
+
+ @Override
+ public LeafReader getLastReader() {
+ return lastReader;
+ }
+ };
}
- leafPtr++;
- }
- lastReader[0] = context.reader();
- lastDoc[0] = -1;
- }
- });
+ @Override
+ public List<LeafReader> reduce(Collection<SimpleCollectorWithLastReader> collectors) {
+ List<LeafReader> lastReaders = new ArrayList<>();
+ for (SimpleCollectorWithLastReader collector : collectors) {
+ lastReaders.add(collector.getLastReader());
Review Comment:
You may want to check if the value returned from `getLastReader` is `null` before adding it to the array (and in the other reduce method).
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Update QueryUtils to use CollectorManager [lucene]
Posted by "javanna (via GitHub)" <gi...@apache.org>.
javanna commented on code in PR #13748:
URL: https://github.com/apache/lucene/pull/13748#discussion_r1752834380
##########
lucene/test-framework/src/java/org/apache/lucene/tests/search/QueryUtils.java:
##########
@@ -345,175 +348,201 @@ public static void checkSkipTo(final Query q, final IndexSearcher s) throws IOEx
// FUTURE: ensure scorer.doc()==-1
final float maxDiff = 1e-5f;
- final LeafReader[] lastReader = {null};
- s.search(
- q,
- new SimpleCollector() {
- private Scorable sc;
- private Scorer scorer;
- private DocIdSetIterator iterator;
- private int leafPtr;
+ List<LeafReader> lastReaders =
+ s.search(
+ q,
+ new CollectorManager<SimpleCollectorWithLastReader, List<LeafReader>>() {
+ @Override
+ public SimpleCollectorWithLastReader newCollector() {
+ return new SimpleCollectorWithLastReader() {
+ LeafReader lastReader = null;
+ private Scorable sc;
+ private Scorer scorer;
+ private DocIdSetIterator iterator;
+ private int leafPtr;
+
+ @Override
+ public void setScorer(Scorable scorer) {
+ this.sc = scorer;
+ }
- @Override
- public void setScorer(Scorable scorer) {
- this.sc = scorer;
- }
+ @Override
+ public void collect(int doc) throws IOException {
+ float score = sc.score();
+ lastDoc[0] = doc;
+ try {
+ if (scorer == null) {
+ Query rewritten = s.rewrite(q);
+ Weight w = s.createWeight(rewritten, ScoreMode.COMPLETE, 1);
+ LeafReaderContext context = readerContextArray.get(leafPtr);
+ scorer = w.scorer(context);
+ iterator = scorer.iterator();
+ }
+
+ int op = order[(opidx[0]++) % order.length];
+ // System.out.println(op==skip_op ?
+ // "skip("+(sdoc[0]+1)+")":"next()");
+ boolean more =
+ op == skip_op
+ ? iterator.advance(scorer.docID() + 1)
+ != DocIdSetIterator.NO_MORE_DOCS
+ : iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS;
+ int scorerDoc = scorer.docID();
+ float scorerScore = scorer.score();
+ float scorerScore2 = scorer.score();
+ float scoreDiff = Math.abs(score - scorerScore);
+ float scorerDiff = Math.abs(scorerScore2 - scorerScore);
+
+ boolean success = false;
+ try {
+ assertTrue(more);
+ assertEquals("scorerDoc=" + scorerDoc + ",doc=" + doc, scorerDoc, doc);
+ assertTrue(
+ "score=" + score + ", scorerScore=" + scorerScore,
+ scoreDiff <= maxDiff);
+ assertTrue(
+ "scorerScorer=" + scorerScore + ", scorerScore2=" + scorerScore2,
+ scorerDiff <= maxDiff);
+ success = true;
+ } finally {
+ if (!success) {
+ if (LuceneTestCase.VERBOSE) {
+ StringBuilder sbord = new StringBuilder();
+ for (int i = 0; i < order.length; i++) {
+ sbord.append(order[i] == skip_op ? " skip()" : " next()");
+ }
+ System.out.println(
+ "ERROR matching docs:"
+ + "\n\t"
+ + (doc != scorerDoc ? "--> " : "")
+ + "doc="
+ + doc
+ + ", scorerDoc="
+ + scorerDoc
+ + "\n\t"
+ + (!more ? "--> " : "")
+ + "tscorer.more="
+ + more
+ + "\n\t"
+ + (scoreDiff > maxDiff ? "--> " : "")
+ + "scorerScore="
+ + scorerScore
+ + " scoreDiff="
+ + scoreDiff
+ + " maxDiff="
+ + maxDiff
+ + "\n\t"
+ + (scorerDiff > maxDiff ? "--> " : "")
+ + "scorerScore2="
+ + scorerScore2
+ + " scorerDiff="
+ + scorerDiff
+ + "\n\thitCollector.doc="
+ + doc
+ + " score="
+ + score
+ + "\n\t Scorer="
+ + scorer
+ + "\n\t Query="
+ + q
+ + " "
+ + q.getClass().getName()
+ + "\n\t Searcher="
+ + s
+ + "\n\t Order="
+ + sbord
+ + "\n\t Op="
+ + (op == skip_op ? " skip()" : " next()"));
+ }
+ }
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
- @Override
- public void collect(int doc) throws IOException {
- float score = sc.score();
- lastDoc[0] = doc;
- try {
- if (scorer == null) {
- Query rewritten = s.rewrite(q);
- Weight w = s.createWeight(rewritten, ScoreMode.COMPLETE, 1);
- LeafReaderContext context = readerContextArray.get(leafPtr);
- scorer = w.scorer(context);
- iterator = scorer.iterator();
- }
+ @Override
+ public ScoreMode scoreMode() {
+ return ScoreMode.COMPLETE;
+ }
- int op = order[(opidx[0]++) % order.length];
- // System.out.println(op==skip_op ?
- // "skip("+(sdoc[0]+1)+")":"next()");
- boolean more =
- op == skip_op
- ? iterator.advance(scorer.docID() + 1) != DocIdSetIterator.NO_MORE_DOCS
- : iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS;
- int scorerDoc = scorer.docID();
- float scorerScore = scorer.score();
- float scorerScore2 = scorer.score();
- float scoreDiff = Math.abs(score - scorerScore);
- float scorerDiff = Math.abs(scorerScore2 - scorerScore);
-
- boolean success = false;
- try {
- assertTrue(more);
- assertEquals("scorerDoc=" + scorerDoc + ",doc=" + doc, scorerDoc, doc);
- assertTrue(
- "score=" + score + ", scorerScore=" + scorerScore, scoreDiff <= maxDiff);
- assertTrue(
- "scorerScorer=" + scorerScore + ", scorerScore2=" + scorerScore2,
- scorerDiff <= maxDiff);
- success = true;
- } finally {
- if (!success) {
- if (LuceneTestCase.VERBOSE) {
- StringBuilder sbord = new StringBuilder();
- for (int i = 0; i < order.length; i++) {
- sbord.append(order[i] == skip_op ? " skip()" : " next()");
+ @Override
+ protected void doSetNextReader(LeafReaderContext context) throws IOException {
+ // confirm that skipping beyond the last doc, on the
+ // previous reader, hits NO_MORE_DOCS
+ if (lastReader != null) {
+ final LeafReader previousReader = lastReader;
+ IndexSearcher indexSearcher =
+ LuceneTestCase.newSearcher(previousReader, false);
+ indexSearcher.setSimilarity(s.getSimilarity());
+ Query rewritten = indexSearcher.rewrite(q);
+ Weight w = indexSearcher.createWeight(rewritten, ScoreMode.COMPLETE, 1);
+ LeafReaderContext ctx =
+ (LeafReaderContext) indexSearcher.getTopReaderContext();
+ Scorer scorer = w.scorer(ctx);
+ if (scorer != null) {
+ DocIdSetIterator iterator = scorer.iterator();
+ boolean more = false;
+ final Bits liveDocs = context.reader().getLiveDocs();
+ for (int d = iterator.advance(lastDoc[0] + 1);
+ d != DocIdSetIterator.NO_MORE_DOCS;
+ d = iterator.nextDoc()) {
+ if (liveDocs == null || liveDocs.get(d)) {
+ more = true;
+ break;
+ }
+ }
+ Assert.assertFalse(
+ "query's last doc was "
+ + lastDoc[0]
+ + " but advance("
+ + (lastDoc[0] + 1)
+ + ") got to "
+ + scorer.docID(),
+ more);
+ }
+ leafPtr++;
}
- System.out.println(
- "ERROR matching docs:"
- + "\n\t"
- + (doc != scorerDoc ? "--> " : "")
- + "doc="
- + doc
- + ", scorerDoc="
- + scorerDoc
- + "\n\t"
- + (!more ? "--> " : "")
- + "tscorer.more="
- + more
- + "\n\t"
- + (scoreDiff > maxDiff ? "--> " : "")
- + "scorerScore="
- + scorerScore
- + " scoreDiff="
- + scoreDiff
- + " maxDiff="
- + maxDiff
- + "\n\t"
- + (scorerDiff > maxDiff ? "--> " : "")
- + "scorerScore2="
- + scorerScore2
- + " scorerDiff="
- + scorerDiff
- + "\n\thitCollector.doc="
- + doc
- + " score="
- + score
- + "\n\t Scorer="
- + scorer
- + "\n\t Query="
- + q
- + " "
- + q.getClass().getName()
- + "\n\t Searcher="
- + s
- + "\n\t Order="
- + sbord
- + "\n\t Op="
- + (op == skip_op ? " skip()" : " next()"));
+ lastReader = context.reader();
+ assert readerContextArray.get(leafPtr).reader() == context.reader();
Review Comment:
I did not have time to review this in depth, but there is more to it than just the assertion. These collectors are rather hard to follow at first glance. Also, this is a case where we cannot resort to a single-threaded collector manager unfortunately, because the searcher is externally provided and that would require disabling concurrency in all those tests that call `QueryUtils` methods somehow. Calling search(Query, Collector) bypassed concurrency so far despite an executor is provided to the searcher, which is why we want to remove it, but that also means that once an executor is provided to the searcher, there is no longer a way to disable search concurrency for specific queries.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
Re: [PR] Update QueryUtils to use CollectorManager [lucene]
Posted by "javanna (via GitHub)" <gi...@apache.org>.
javanna commented on code in PR #13748:
URL: https://github.com/apache/lucene/pull/13748#discussion_r1752834380
##########
lucene/test-framework/src/java/org/apache/lucene/tests/search/QueryUtils.java:
##########
@@ -345,175 +348,201 @@ public static void checkSkipTo(final Query q, final IndexSearcher s) throws IOEx
// FUTURE: ensure scorer.doc()==-1
final float maxDiff = 1e-5f;
- final LeafReader[] lastReader = {null};
- s.search(
- q,
- new SimpleCollector() {
- private Scorable sc;
- private Scorer scorer;
- private DocIdSetIterator iterator;
- private int leafPtr;
+ List<LeafReader> lastReaders =
+ s.search(
+ q,
+ new CollectorManager<SimpleCollectorWithLastReader, List<LeafReader>>() {
+ @Override
+ public SimpleCollectorWithLastReader newCollector() {
+ return new SimpleCollectorWithLastReader() {
+ LeafReader lastReader = null;
+ private Scorable sc;
+ private Scorer scorer;
+ private DocIdSetIterator iterator;
+ private int leafPtr;
+
+ @Override
+ public void setScorer(Scorable scorer) {
+ this.sc = scorer;
+ }
- @Override
- public void setScorer(Scorable scorer) {
- this.sc = scorer;
- }
+ @Override
+ public void collect(int doc) throws IOException {
+ float score = sc.score();
+ lastDoc[0] = doc;
+ try {
+ if (scorer == null) {
+ Query rewritten = s.rewrite(q);
+ Weight w = s.createWeight(rewritten, ScoreMode.COMPLETE, 1);
+ LeafReaderContext context = readerContextArray.get(leafPtr);
+ scorer = w.scorer(context);
+ iterator = scorer.iterator();
+ }
+
+ int op = order[(opidx[0]++) % order.length];
+ // System.out.println(op==skip_op ?
+ // "skip("+(sdoc[0]+1)+")":"next()");
+ boolean more =
+ op == skip_op
+ ? iterator.advance(scorer.docID() + 1)
+ != DocIdSetIterator.NO_MORE_DOCS
+ : iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS;
+ int scorerDoc = scorer.docID();
+ float scorerScore = scorer.score();
+ float scorerScore2 = scorer.score();
+ float scoreDiff = Math.abs(score - scorerScore);
+ float scorerDiff = Math.abs(scorerScore2 - scorerScore);
+
+ boolean success = false;
+ try {
+ assertTrue(more);
+ assertEquals("scorerDoc=" + scorerDoc + ",doc=" + doc, scorerDoc, doc);
+ assertTrue(
+ "score=" + score + ", scorerScore=" + scorerScore,
+ scoreDiff <= maxDiff);
+ assertTrue(
+ "scorerScorer=" + scorerScore + ", scorerScore2=" + scorerScore2,
+ scorerDiff <= maxDiff);
+ success = true;
+ } finally {
+ if (!success) {
+ if (LuceneTestCase.VERBOSE) {
+ StringBuilder sbord = new StringBuilder();
+ for (int i = 0; i < order.length; i++) {
+ sbord.append(order[i] == skip_op ? " skip()" : " next()");
+ }
+ System.out.println(
+ "ERROR matching docs:"
+ + "\n\t"
+ + (doc != scorerDoc ? "--> " : "")
+ + "doc="
+ + doc
+ + ", scorerDoc="
+ + scorerDoc
+ + "\n\t"
+ + (!more ? "--> " : "")
+ + "tscorer.more="
+ + more
+ + "\n\t"
+ + (scoreDiff > maxDiff ? "--> " : "")
+ + "scorerScore="
+ + scorerScore
+ + " scoreDiff="
+ + scoreDiff
+ + " maxDiff="
+ + maxDiff
+ + "\n\t"
+ + (scorerDiff > maxDiff ? "--> " : "")
+ + "scorerScore2="
+ + scorerScore2
+ + " scorerDiff="
+ + scorerDiff
+ + "\n\thitCollector.doc="
+ + doc
+ + " score="
+ + score
+ + "\n\t Scorer="
+ + scorer
+ + "\n\t Query="
+ + q
+ + " "
+ + q.getClass().getName()
+ + "\n\t Searcher="
+ + s
+ + "\n\t Order="
+ + sbord
+ + "\n\t Op="
+ + (op == skip_op ? " skip()" : " next()"));
+ }
+ }
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
- @Override
- public void collect(int doc) throws IOException {
- float score = sc.score();
- lastDoc[0] = doc;
- try {
- if (scorer == null) {
- Query rewritten = s.rewrite(q);
- Weight w = s.createWeight(rewritten, ScoreMode.COMPLETE, 1);
- LeafReaderContext context = readerContextArray.get(leafPtr);
- scorer = w.scorer(context);
- iterator = scorer.iterator();
- }
+ @Override
+ public ScoreMode scoreMode() {
+ return ScoreMode.COMPLETE;
+ }
- int op = order[(opidx[0]++) % order.length];
- // System.out.println(op==skip_op ?
- // "skip("+(sdoc[0]+1)+")":"next()");
- boolean more =
- op == skip_op
- ? iterator.advance(scorer.docID() + 1) != DocIdSetIterator.NO_MORE_DOCS
- : iterator.nextDoc() != DocIdSetIterator.NO_MORE_DOCS;
- int scorerDoc = scorer.docID();
- float scorerScore = scorer.score();
- float scorerScore2 = scorer.score();
- float scoreDiff = Math.abs(score - scorerScore);
- float scorerDiff = Math.abs(scorerScore2 - scorerScore);
-
- boolean success = false;
- try {
- assertTrue(more);
- assertEquals("scorerDoc=" + scorerDoc + ",doc=" + doc, scorerDoc, doc);
- assertTrue(
- "score=" + score + ", scorerScore=" + scorerScore, scoreDiff <= maxDiff);
- assertTrue(
- "scorerScorer=" + scorerScore + ", scorerScore2=" + scorerScore2,
- scorerDiff <= maxDiff);
- success = true;
- } finally {
- if (!success) {
- if (LuceneTestCase.VERBOSE) {
- StringBuilder sbord = new StringBuilder();
- for (int i = 0; i < order.length; i++) {
- sbord.append(order[i] == skip_op ? " skip()" : " next()");
+ @Override
+ protected void doSetNextReader(LeafReaderContext context) throws IOException {
+ // confirm that skipping beyond the last doc, on the
+ // previous reader, hits NO_MORE_DOCS
+ if (lastReader != null) {
+ final LeafReader previousReader = lastReader;
+ IndexSearcher indexSearcher =
+ LuceneTestCase.newSearcher(previousReader, false);
+ indexSearcher.setSimilarity(s.getSimilarity());
+ Query rewritten = indexSearcher.rewrite(q);
+ Weight w = indexSearcher.createWeight(rewritten, ScoreMode.COMPLETE, 1);
+ LeafReaderContext ctx =
+ (LeafReaderContext) indexSearcher.getTopReaderContext();
+ Scorer scorer = w.scorer(ctx);
+ if (scorer != null) {
+ DocIdSetIterator iterator = scorer.iterator();
+ boolean more = false;
+ final Bits liveDocs = context.reader().getLiveDocs();
+ for (int d = iterator.advance(lastDoc[0] + 1);
+ d != DocIdSetIterator.NO_MORE_DOCS;
+ d = iterator.nextDoc()) {
+ if (liveDocs == null || liveDocs.get(d)) {
+ more = true;
+ break;
+ }
+ }
+ Assert.assertFalse(
+ "query's last doc was "
+ + lastDoc[0]
+ + " but advance("
+ + (lastDoc[0] + 1)
+ + ") got to "
+ + scorer.docID(),
+ more);
+ }
+ leafPtr++;
}
- System.out.println(
- "ERROR matching docs:"
- + "\n\t"
- + (doc != scorerDoc ? "--> " : "")
- + "doc="
- + doc
- + ", scorerDoc="
- + scorerDoc
- + "\n\t"
- + (!more ? "--> " : "")
- + "tscorer.more="
- + more
- + "\n\t"
- + (scoreDiff > maxDiff ? "--> " : "")
- + "scorerScore="
- + scorerScore
- + " scoreDiff="
- + scoreDiff
- + " maxDiff="
- + maxDiff
- + "\n\t"
- + (scorerDiff > maxDiff ? "--> " : "")
- + "scorerScore2="
- + scorerScore2
- + " scorerDiff="
- + scorerDiff
- + "\n\thitCollector.doc="
- + doc
- + " score="
- + score
- + "\n\t Scorer="
- + scorer
- + "\n\t Query="
- + q
- + " "
- + q.getClass().getName()
- + "\n\t Searcher="
- + s
- + "\n\t Order="
- + sbord
- + "\n\t Op="
- + (op == skip_op ? " skip()" : " next()"));
+ lastReader = context.reader();
+ assert readerContextArray.get(leafPtr).reader() == context.reader();
Review Comment:
I did not have time to review this in depth, but there is more to it than just the assertion. These collectors are rather hard to follow at first glance. Also, this is a case where we cannot resort to a single-threaded collector manager unfortunately, because the searcher is externally provided and that would require disabling concurrency in all those tests that call `QueryUtils` methods somehow. Calling search(Query, Collector) bypassed concurrency so far despite an executor is provided to the searcher, which is why we want to remove it.
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org