You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/08/04 18:46:43 UTC
[GitHub] [lucene] gsmiller opened a new pull request, #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery
gsmiller opened a new pull request, #1058:
URL: https://github.com/apache/lucene/pull/1058
This makes incremental progress against LUCENE-10207, allowing `TermInSetQuery` to provide cost estimation so it might be used in an `IndexOrDocValuesQuery`.
--
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
[GitHub] [lucene] msokolov commented on a diff in pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery
Posted by GitBox <gi...@apache.org>.
msokolov commented on code in PR #1058:
URL: https://github.com/apache/lucene/pull/1058#discussion_r950508835
##########
lucene/core/src/java/org/apache/lucene/search/TermInSetQuery.java:
##########
@@ -345,15 +345,62 @@ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException {
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- final WeightOrDocIdSet weightOrBitSet = rewrite(context);
- if (weightOrBitSet == null) {
- return null;
- } else if (weightOrBitSet.weight != null) {
- return weightOrBitSet.weight.scorer(context);
- } else {
- return scorer(weightOrBitSet.set);
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+ // Cost estimation reasoning is:
+ // 1. Assume every query term matches at least one document (queryTermsCount).
+ // 2. Determine the total number of docs beyond the first one for each term.
+ // That count provides a ceiling on the number of extra docs that could match beyond
+ // that first one. (We omit the first since it's already been counted in #1).
+ // This approach still provides correct worst-case cost in general, but provides tighter
+ // estimates for primary-key-like fields. See: LUCENE-10207
+
+ // TODO: This cost estimation may grossly overestimate since we have no index statistics
+ // for the specific query terms. While it's nice to avoid the cost of intersecting the
+ // query terms with the index, it could be beneficial to do that work and get better
+ // cost estimates.
+ final long cost;
+ final long queryTermsCount = termData.size();
+ Terms indexTerms = context.reader().terms(field);
+ long potentialExtraCost = indexTerms.getSumDocFreq();
+ final long indexedTermCount = indexTerms.size();
+ if (indexedTermCount != -1) {
+ potentialExtraCost -= indexedTermCount;
}
+ cost = queryTermsCount + potentialExtraCost;
+
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ WeightOrDocIdSet weightOrDocIdSet = rewrite(context);
+ if (weightOrDocIdSet == null) {
+ return null;
+ }
+
+ final Scorer scorer;
+ if (weightOrDocIdSet.weight != null) {
+ scorer = weightOrDocIdSet.weight.scorer(context);
+ } else {
+ scorer = scorer(weightOrDocIdSet.set);
+ }
+
+ return Objects.requireNonNullElseGet(
+ scorer,
+ () ->
+ new ConstantScoreScorer(weight, score(), scoreMode, DocIdSetIterator.empty()));
+ }
+
+ @Override
+ public long cost() {
+ return cost;
Review Comment:
yeah that's fine. I guess I thought it had to with number of documents, but if it is just sort of abstractly "how expensive is this" like in terms of operations then it makes sense to let it blow up
--
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
[GitHub] [lucene] gsmiller commented on a diff in pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #1058:
URL: https://github.com/apache/lucene/pull/1058#discussion_r951767154
##########
lucene/core/src/java/org/apache/lucene/search/TermInSetQuery.java:
##########
@@ -345,15 +345,62 @@ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException {
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- final WeightOrDocIdSet weightOrBitSet = rewrite(context);
- if (weightOrBitSet == null) {
- return null;
- } else if (weightOrBitSet.weight != null) {
- return weightOrBitSet.weight.scorer(context);
- } else {
- return scorer(weightOrBitSet.set);
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+ // Cost estimation reasoning is:
+ // 1. Assume every query term matches at least one document (queryTermsCount).
+ // 2. Determine the total number of docs beyond the first one for each term.
+ // That count provides a ceiling on the number of extra docs that could match beyond
+ // that first one. (We omit the first since it's already been counted in #1).
+ // This approach still provides correct worst-case cost in general, but provides tighter
+ // estimates for primary-key-like fields. See: LUCENE-10207
+
+ // TODO: This cost estimation may grossly overestimate since we have no index statistics
+ // for the specific query terms. While it's nice to avoid the cost of intersecting the
+ // query terms with the index, it could be beneficial to do that work and get better
+ // cost estimates.
+ final long cost;
+ final long queryTermsCount = termData.size();
+ Terms indexTerms = context.reader().terms(field);
+ long potentialExtraCost = indexTerms.getSumDocFreq();
+ final long indexedTermCount = indexTerms.size();
+ if (indexedTermCount != -1) {
+ potentialExtraCost -= indexedTermCount;
}
+ cost = queryTermsCount + potentialExtraCost;
+
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ WeightOrDocIdSet weightOrDocIdSet = rewrite(context);
+ if (weightOrDocIdSet == null) {
+ return null;
+ }
+
+ final Scorer scorer;
+ if (weightOrDocIdSet.weight != null) {
+ scorer = weightOrDocIdSet.weight.scorer(context);
+ } else {
+ scorer = scorer(weightOrDocIdSet.set);
+ }
+
+ return Objects.requireNonNullElseGet(
+ scorer,
+ () ->
+ new ConstantScoreScorer(weight, score(), scoreMode, DocIdSetIterator.empty()));
+ }
+
+ @Override
+ public long cost() {
+ return cost;
Review Comment:
Oh, and I should clarify that I referenced the `DocIdSetIterator#cost` javadoc above since the `ScoreSupplier#cost` javadoc doesn't really provide guidance on the meaning but instead references DISI#cost.
--
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
[GitHub] [lucene] gsmiller commented on a diff in pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #1058:
URL: https://github.com/apache/lucene/pull/1058#discussion_r951744466
##########
lucene/core/src/java/org/apache/lucene/search/TermInSetQuery.java:
##########
@@ -345,15 +345,62 @@ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException {
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- final WeightOrDocIdSet weightOrBitSet = rewrite(context);
- if (weightOrBitSet == null) {
- return null;
- } else if (weightOrBitSet.weight != null) {
- return weightOrBitSet.weight.scorer(context);
- } else {
- return scorer(weightOrBitSet.set);
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+ // Cost estimation reasoning is:
+ // 1. Assume every query term matches at least one document (queryTermsCount).
+ // 2. Determine the total number of docs beyond the first one for each term.
+ // That count provides a ceiling on the number of extra docs that could match beyond
+ // that first one. (We omit the first since it's already been counted in #1).
+ // This approach still provides correct worst-case cost in general, but provides tighter
+ // estimates for primary-key-like fields. See: LUCENE-10207
+
+ // TODO: This cost estimation may grossly overestimate since we have no index statistics
+ // for the specific query terms. While it's nice to avoid the cost of intersecting the
+ // query terms with the index, it could be beneficial to do that work and get better
+ // cost estimates.
+ final long cost;
+ final long queryTermsCount = termData.size();
+ Terms indexTerms = context.reader().terms(field);
+ long potentialExtraCost = indexTerms.getSumDocFreq();
+ final long indexedTermCount = indexTerms.size();
+ if (indexedTermCount != -1) {
+ potentialExtraCost -= indexedTermCount;
}
+ cost = queryTermsCount + potentialExtraCost;
+
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ WeightOrDocIdSet weightOrDocIdSet = rewrite(context);
+ if (weightOrDocIdSet == null) {
+ return null;
+ }
+
+ final Scorer scorer;
+ if (weightOrDocIdSet.weight != null) {
+ scorer = weightOrDocIdSet.weight.scorer(context);
+ } else {
+ scorer = scorer(weightOrDocIdSet.set);
+ }
+
+ return Objects.requireNonNullElseGet(
+ scorer,
+ () ->
+ new ConstantScoreScorer(weight, score(), scoreMode, DocIdSetIterator.empty()));
+ }
+
+ @Override
+ public long cost() {
+ return cost;
Review Comment:
@msokolov well, according to the javadoc, it can be number of documents or basically any other number you might want to choose that may or may not have significance :). Kidding. Sort of. Here's the javadoc:
Joking aside, I think being consistent with how disjunctions in general estimate cost makes sense for now at least.
```
/**
* Returns the estimated cost of this {@link DocIdSetIterator}.
*
* <p>This is generally an upper bound of the number of documents this iterator might match, but
* may be a rough heuristic, hardcoded value, or otherwise completely inaccurate.
*/
```
--
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
[GitHub] [lucene] gsmiller commented on pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in IndexOrDocValuesQuery
Posted by GitBox <gi...@apache.org>.
gsmiller commented on PR #1058:
URL: https://github.com/apache/lucene/pull/1058#issuecomment-1234812574
Thanks @msokolov. Merged and backported.
--
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
[GitHub] [lucene] gsmiller merged pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in IndexOrDocValuesQuery
Posted by GitBox <gi...@apache.org>.
gsmiller merged PR #1058:
URL: https://github.com/apache/lucene/pull/1058
--
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
[GitHub] [lucene] msokolov commented on a diff in pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery
Posted by GitBox <gi...@apache.org>.
msokolov commented on code in PR #1058:
URL: https://github.com/apache/lucene/pull/1058#discussion_r950509327
##########
lucene/CHANGES.txt:
##########
@@ -95,6 +95,9 @@ Improvements
---------------------
* LUCENE-10592: Build HNSW Graph on indexing. (Mayya Sharipova, Adrien Grand, Julie Tibshirani)
+* LUCENE-10207: TermInSetQuery can now provide a ScoreSupplier with cost estimation, making it
+ usable in TermInSetQuery. (Greg Miller)
Review Comment:
this comment is confusing -- do you really mean that now TISQ is usable in TISQ?
--
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
[GitHub] [lucene] gsmiller commented on a diff in pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #1058:
URL: https://github.com/apache/lucene/pull/1058#discussion_r940164398
##########
lucene/core/src/java/org/apache/lucene/search/TermInSetQuery.java:
##########
@@ -345,15 +345,62 @@ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException {
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- final WeightOrDocIdSet weightOrBitSet = rewrite(context);
- if (weightOrBitSet == null) {
- return null;
- } else if (weightOrBitSet.weight != null) {
- return weightOrBitSet.weight.scorer(context);
- } else {
- return scorer(weightOrBitSet.set);
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+ // Cost estimation reasoning is:
+ // 1. Assume every query term matches at least one document (queryTermsCount).
+ // 2. Determine the total number of docs beyond the first one for each term.
+ // That count provides a ceiling on the number of extra docs that could match beyond
+ // that first one. (We omit the first since it's already been counted in #1).
+ // This approach still provides correct worst-case cost in general, but provides tighter
+ // estimates for primary-key-like fields. See: LUCENE-10207
+
+ // TODO: This cost estimation may grossly overestimate since we have no index statistics
+ // for the specific query terms. While it's nice to avoid the cost of intersecting the
+ // query terms with the index, it could be beneficial to do that work and get better
+ // cost estimates.
+ final long cost;
+ final long queryTermsCount = termData.size();
+ Terms indexTerms = context.reader().terms(field);
+ long potentialExtraCost = indexTerms.getSumDocFreq();
+ final long indexedTermCount = indexTerms.size();
+ if (indexedTermCount != -1) {
+ potentialExtraCost -= indexedTermCount;
}
+ cost = queryTermsCount + potentialExtraCost;
+
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ WeightOrDocIdSet weightOrDocIdSet = rewrite(context);
+ if (weightOrDocIdSet == null) {
+ return null;
+ }
+
+ final Scorer scorer;
+ if (weightOrDocIdSet.weight != null) {
+ scorer = weightOrDocIdSet.weight.scorer(context);
+ } else {
+ scorer = scorer(weightOrDocIdSet.set);
+ }
+
+ return Objects.requireNonNullElseGet(
+ scorer,
+ () ->
+ new ConstantScoreScorer(weight, score(), scoreMode, DocIdSetIterator.empty()));
+ }
+
+ @Override
+ public long cost() {
+ return cost;
Review Comment:
I went this direction initially, but [got some feedback](https://issues.apache.org/jira/browse/LUCENE-10207?focusedCommentId=17436292&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17436292) that we expect `#cost()` to model the amount of work required by the scorer, which in the case of a disjunction like this, is more proportional to `sumDocFreq`. I suppose it's still not 100% clear to me if `#cost` should model the number of docs that could be produced or the amount of work. But if you dig into [the way we estimate cost for disjunctions in general](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/search/ScorerUtil.java#L47), it doesn't put any cap on cost, so this is consistent.
--
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
[GitHub] [lucene] gsmiller commented on a diff in pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #1058:
URL: https://github.com/apache/lucene/pull/1058#discussion_r951745046
##########
lucene/CHANGES.txt:
##########
@@ -95,6 +95,9 @@ Improvements
---------------------
* LUCENE-10592: Build HNSW Graph on indexing. (Mayya Sharipova, Adrien Grand, Julie Tibshirani)
+* LUCENE-10207: TermInSetQuery can now provide a ScoreSupplier with cost estimation, making it
+ usable in TermInSetQuery. (Greg Miller)
Review Comment:
Oops. I meant to write that it's usable in `IndexOrDocValuesQuery`. Doh! Let me fix. Thanks for the catch.
--
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
[GitHub] [lucene] msokolov commented on a diff in pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery
Posted by GitBox <gi...@apache.org>.
msokolov commented on code in PR #1058:
URL: https://github.com/apache/lucene/pull/1058#discussion_r939587786
##########
lucene/core/src/java/org/apache/lucene/search/TermInSetQuery.java:
##########
@@ -345,15 +345,62 @@ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException {
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- final WeightOrDocIdSet weightOrBitSet = rewrite(context);
- if (weightOrBitSet == null) {
- return null;
- } else if (weightOrBitSet.weight != null) {
- return weightOrBitSet.weight.scorer(context);
- } else {
- return scorer(weightOrBitSet.set);
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+ // Cost estimation reasoning is:
+ // 1. Assume every query term matches at least one document (queryTermsCount).
+ // 2. Determine the total number of docs beyond the first one for each term.
+ // That count provides a ceiling on the number of extra docs that could match beyond
+ // that first one. (We omit the first since it's already been counted in #1).
+ // This approach still provides correct worst-case cost in general, but provides tighter
+ // estimates for primary-key-like fields. See: LUCENE-10207
+
+ // TODO: This cost estimation may grossly overestimate since we have no index statistics
+ // for the specific query terms. While it's nice to avoid the cost of intersecting the
+ // query terms with the index, it could be beneficial to do that work and get better
+ // cost estimates.
+ final long cost;
+ final long queryTermsCount = termData.size();
+ Terms indexTerms = context.reader().terms(field);
+ long potentialExtraCost = indexTerms.getSumDocFreq();
+ final long indexedTermCount = indexTerms.size();
+ if (indexedTermCount != -1) {
+ potentialExtraCost -= indexedTermCount;
}
+ cost = queryTermsCount + potentialExtraCost;
+
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ WeightOrDocIdSet weightOrDocIdSet = rewrite(context);
+ if (weightOrDocIdSet == null) {
+ return null;
+ }
+
+ final Scorer scorer;
+ if (weightOrDocIdSet.weight != null) {
+ scorer = weightOrDocIdSet.weight.scorer(context);
+ } else {
+ scorer = scorer(weightOrDocIdSet.set);
+ }
+
+ return Objects.requireNonNullElseGet(
+ scorer,
+ () ->
+ new ConstantScoreScorer(weight, score(), scoreMode, DocIdSetIterator.empty()));
+ }
+
+ @Override
+ public long cost() {
+ return cost;
Review Comment:
should we bound this by `maxDoc`? I guess we could have computed something bigger if there are multiple terms in some docs.
--
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
[GitHub] [lucene] gsmiller commented on pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in IndexOrDocValuesQuery
Posted by GitBox <gi...@apache.org>.
gsmiller commented on PR #1058:
URL: https://github.com/apache/lucene/pull/1058#issuecomment-1234688519
@msokolov any additional feedback or concerns on this? If not, I'll merge today so it can go with 9.4. It's not critical to get it into 9.4 though, so if you (or anyone else) would like some extra time to consider the change, I can wait on it. Thanks!
--
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
[GitHub] [lucene] gsmiller commented on a diff in pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #1058:
URL: https://github.com/apache/lucene/pull/1058#discussion_r948265844
##########
lucene/core/src/java/org/apache/lucene/search/TermInSetQuery.java:
##########
@@ -345,15 +345,62 @@ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException {
}
@Override
- public Scorer scorer(LeafReaderContext context) throws IOException {
- final WeightOrDocIdSet weightOrBitSet = rewrite(context);
- if (weightOrBitSet == null) {
- return null;
- } else if (weightOrBitSet.weight != null) {
- return weightOrBitSet.weight.scorer(context);
- } else {
- return scorer(weightOrBitSet.set);
+ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOException {
+ // Cost estimation reasoning is:
+ // 1. Assume every query term matches at least one document (queryTermsCount).
+ // 2. Determine the total number of docs beyond the first one for each term.
+ // That count provides a ceiling on the number of extra docs that could match beyond
+ // that first one. (We omit the first since it's already been counted in #1).
+ // This approach still provides correct worst-case cost in general, but provides tighter
+ // estimates for primary-key-like fields. See: LUCENE-10207
+
+ // TODO: This cost estimation may grossly overestimate since we have no index statistics
+ // for the specific query terms. While it's nice to avoid the cost of intersecting the
+ // query terms with the index, it could be beneficial to do that work and get better
+ // cost estimates.
+ final long cost;
+ final long queryTermsCount = termData.size();
+ Terms indexTerms = context.reader().terms(field);
+ long potentialExtraCost = indexTerms.getSumDocFreq();
+ final long indexedTermCount = indexTerms.size();
+ if (indexedTermCount != -1) {
+ potentialExtraCost -= indexedTermCount;
}
+ cost = queryTermsCount + potentialExtraCost;
+
+ final Weight weight = this;
+ return new ScorerSupplier() {
+ @Override
+ public Scorer get(long leadCost) throws IOException {
+ WeightOrDocIdSet weightOrDocIdSet = rewrite(context);
+ if (weightOrDocIdSet == null) {
+ return null;
+ }
+
+ final Scorer scorer;
+ if (weightOrDocIdSet.weight != null) {
+ scorer = weightOrDocIdSet.weight.scorer(context);
+ } else {
+ scorer = scorer(weightOrDocIdSet.set);
+ }
+
+ return Objects.requireNonNullElseGet(
+ scorer,
+ () ->
+ new ConstantScoreScorer(weight, score(), scoreMode, DocIdSetIterator.empty()));
+ }
+
+ @Override
+ public long cost() {
+ return cost;
Review Comment:
@msokolov when you have a chance, I'm curious what you think about this ^^. Thanks!
--
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
[GitHub] [lucene] gsmiller commented on a diff in pull request #1058: LUCENE-10207: TermInSetQuery now provides a ScoreSupplier with cost estimation for use in TermInSetQuery
Posted by GitBox <gi...@apache.org>.
gsmiller commented on code in PR #1058:
URL: https://github.com/apache/lucene/pull/1058#discussion_r951765243
##########
lucene/CHANGES.txt:
##########
@@ -95,6 +95,9 @@ Improvements
---------------------
* LUCENE-10592: Build HNSW Graph on indexing. (Mayya Sharipova, Adrien Grand, Julie Tibshirani)
+* LUCENE-10207: TermInSetQuery can now provide a ScoreSupplier with cost estimation, making it
+ usable in TermInSetQuery. (Greg Miller)
Review Comment:
OK, fixed this. Looks like I also made the same slip-up mistake in my git commit history too. I'll make sure I clean that up before merging.
--
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