You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "sijuv (via GitHub)" <gi...@apache.org> on 2025/02/26 18:15:56 UTC
[PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
sijuv opened a new pull request, #3222:
URL: https://github.com/apache/solr/pull/3222
…erank query.
https://issues.apache.org/jira/browse/SOLR-17678
# Description
Add support for returning original query score for reranked docs.
# Solution
- Carry fwd the score of the scored docs to the rerank phase and return as _matchScore_ in the response if this field is requested.
-
# Tests
Added additional tests to verify matchScore being returned. Also modified test cases for refactoring around _DocSlice_
# Checklist
Please review the following and check all that apply:
- [ X] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability.
- [X ] I have created a Jira issue and added the issue ID to my pull request title.
- [X ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
- [X ] I have developed this patch against the `main` branch.
- [X ] I have run `./gradlew check`.
- [ X] I have added tests for my changes.
- [ X] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "sijuv (via GitHub)" <gi...@apache.org>.
sijuv commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r1984121100
##########
solr/core/src/test/org/apache/solr/search/TestReRankQParserPlugin.java:
##########
@@ -64,6 +64,80 @@ public void testReRankQParserPluginConstants() {
assertEquals(ReRankQParserPlugin.RERANK_OPERATOR, "reRankOperator");
}
+ @Test
+ public void testRerankReturnOriginalScore() throws Exception {
Review Comment:
updated
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2124888615
##########
solr/core/src/java/org/apache/solr/search/DocIterator.java:
##########
@@ -24,7 +24,7 @@
* <p>The order of the documents is determined by the context in which the DocIterator instance was
* retrieved.
*/
-public interface DocIterator extends Iterator<Integer> {
+public interface DocIterator extends Iterator<Integer>, DocIterationInfo {
Review Comment:
We can certainly add `hasScore()` as well. But I think the `DocIterationInfo` is a useful interface, so that we can cleanly add new inputs to DocTransformers in the future.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2124890606
##########
solr/solrj/src/java/org/apache/solr/common/SolrDocument.java:
##########
@@ -195,6 +197,19 @@ public Collection<Object> getFieldValues(String name) {
return null;
}
+ /** Get the value or collection of values for a given field. */
+ public Map<String, Object> getSubsetOfFields(Set<String> fieldNames) {
Review Comment:
Fair enough, this could be removed and put into the QueryComponent, since it's the only place that uses 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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #3222:
URL: https://github.com/apache/solr/pull/3222#issuecomment-2779275057
I've added https://issues.apache.org/jira/browse/SOLR-17732 to account for the relatively big code change to support distributed score fields.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #3222:
URL: https://github.com/apache/solr/pull/3222#issuecomment-2777236714
To be explicit about the large amount of code changed here, there was no support for non-"score" named score return fields, such as "temp:score" (aliased return fields), in distributed requests. This meant that `matchScore` was also not supported. So I went ahead and added support for score return fields in distributed requests.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2125495095
##########
solr/core/src/java/org/apache/solr/search/SolrReturnFields.java:
##########
@@ -50,6 +51,7 @@
public class SolrReturnFields extends ReturnFields {
// Special Field Keys
public static final String SCORE = "score";
+ public static final String MATCH_SCORE = "matchScore";
Review Comment:
I think this part of the change bothers me the most so I'm looking forward to PR3340 to hopefully make this obsolete.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "sijuv (via GitHub)" <gi...@apache.org>.
sijuv commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r1983830223
##########
solr/core/src/java/org/apache/solr/search/DocIterator.java:
##########
@@ -43,4 +43,13 @@ public interface DocIterator extends Iterator<Integer> {
* instance was retrieved.
*/
public float score();
+
+ /**
+ * Returns the query match score in case of rerank queries
+ *
+ * @return the query match score in case of a rerank query, null otherwise.
+ */
+ public default Float matchScore() {
+ return null;
+ }
Review Comment:
the issue is that the matchScore is only relevant if there is a rerank in question. We cannot return 0 and rely on this being 0 to not populate the field because 0 could be a valid score. The scores would depend on how the user formed the scoring query.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2129733214
##########
solr/core/src/java/org/apache/solr/search/SolrReturnFields.java:
##########
@@ -50,6 +51,7 @@
public class SolrReturnFields extends ReturnFields {
// Special Field Keys
public static final String SCORE = "score";
+ public static final String MATCH_SCORE = "matchScore";
Review Comment:
So we _could_ set this to be "matchScore()" as a special case, and eventually change it to be a function when that can be supported better.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "sijuv (via GitHub)" <gi...@apache.org>.
sijuv commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2017841694
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -1984,18 +1982,22 @@ public ScoreMode scoreMode() {
populateNextCursorMarkFromTopDocs(qr, cmd, topDocs);
nDocsReturned = topDocs.scoreDocs.length;
- ids = new int[nDocsReturned];
- scores = needScores ? new float[nDocsReturned] : null;
- for (int i = 0; i < nDocsReturned; i++) {
- ScoreDoc scoreDoc = topDocs.scoreDocs[i];
- ids[i] = scoreDoc.doc;
- if (scores != null) scores[i] = scoreDoc.score;
+ int sliceLen = Math.min(lastDocRequested, nDocsReturned);
+ if (sliceLen < 0) {
+ sliceLen = 0;
+ }
+ if (!needScores) {
+ int[] ids = new int[nDocsReturned];
+ for (int i = 0; i < nDocsReturned; i++) {
+ ScoreDoc scoreDoc = topDocs.scoreDocs[i];
+ ids[i] = scoreDoc.doc;
+ }
+ docList = new DocSlice(0, sliceLen, ids, null, totalHits, maxScore, hitsRelation);
+ } else {
+ docList = new TopDocsSlice(0, sliceLen, topDocs, totalHits, maxScore, hitsRelation);
}
Review Comment:
yes we could. however I was not sure if there are places where we don't need scores and we dont use the `DocTransformer` to populate the results. Esp since `needScores` was based on a condition which was not a function of requested fields.
` final boolean needScores = (cmd.getFlags() & GET_SCORES) != 0;
`
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2035795628
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -1984,18 +1982,22 @@ public ScoreMode scoreMode() {
populateNextCursorMarkFromTopDocs(qr, cmd, topDocs);
nDocsReturned = topDocs.scoreDocs.length;
- ids = new int[nDocsReturned];
- scores = needScores ? new float[nDocsReturned] : null;
- for (int i = 0; i < nDocsReturned; i++) {
- ScoreDoc scoreDoc = topDocs.scoreDocs[i];
- ids[i] = scoreDoc.doc;
- if (scores != null) scores[i] = scoreDoc.score;
+ int sliceLen = Math.min(lastDocRequested, nDocsReturned);
+ if (sliceLen < 0) {
+ sliceLen = 0;
+ }
+ if (!needScores) {
+ int[] ids = new int[nDocsReturned];
+ for (int i = 0; i < nDocsReturned; i++) {
+ ScoreDoc scoreDoc = topDocs.scoreDocs[i];
+ ids[i] = scoreDoc.doc;
+ }
+ docList = new DocSlice(0, sliceLen, ids, null, totalHits, maxScore, hitsRelation);
+ } else {
+ docList = new TopDocsSlice(0, sliceLen, topDocs, totalHits, maxScore, hitsRelation);
}
Review Comment:
Yeah, I agree that `needScores` is not really a good bearing on whether scores exist or not. Anyways, that's the method that was used before, so we can keep using it for now.
I'm adding a `hasScores` variable in `TopDocsSlice`, so we can just use that for all of these instances now. Should be a performance improvement. Plus the code is easier to understand.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r1983473324
##########
solr/core/src/java/org/apache/solr/search/DocIterator.java:
##########
@@ -43,4 +43,13 @@ public interface DocIterator extends Iterator<Integer> {
* instance was retrieved.
*/
public float score();
+
+ /**
+ * Returns the query match score in case of rerank queries
+ *
+ * @return the query match score in case of a rerank query, null otherwise.
+ */
+ public default Float matchScore() {
+ return null;
+ }
Review Comment:
Much like `score()` we might want to keep this as a `float` and return 0. Not really leaning either way on this yet.
##########
solr/core/src/java/org/apache/solr/search/DocSlice.java:
##########
Review Comment:
For this, would it make more sense to have another implementation of DocSlice for a TopDocs? It looks like every method has an if statement in it. Might be more efficient to go with two implementations.
##########
solr/solr-ref-guide/modules/query-guide/pages/query-re-ranking.adoc:
##########
@@ -107,6 +107,9 @@ q=greetings&rq={!rerank reRankQuery=$rqq reRankDocs=1000 reRankWeight=3}&rqq=(hi
----
If a document matches the original query, but does not match the re-ranking query, the document's original score will remain.
+For reranked documents, an additional _matchScore_ field in the response will indicate the original score for a reranked doc. This
+is the score for the document prior to rerank being applied. For documents that were not reranked, the matchScore and score fields
+will have the same value.
Review Comment:
```suggestion
For reranked documents, an additional `matchScore` field in the response will indicate the original score for a reranked doc. This
is the score for the document prior to rerank being applied. For documents that were not reranked, the matchScore and score fields
will have the same value. For the example above, you would use the following to return the match score:
[source,text]
----
q=greetings&rq={!rerank reRankQuery=$rqq reRankDocs=1000 reRankWeight=3}&rqq=(hi+hello+hey+hiya)&fl=id,matchScore
----
```
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
Review Comment:
This in general looks fine to me, but I am not an expert here and would love another set of eyes.
##########
solr/core/src/java/org/apache/solr/search/ReRankCollector.java:
##########
@@ -205,18 +205,29 @@ public TopDocs topDocs(int start, int howMany) {
ScoreDoc[] scoreDocs = new ScoreDoc[howMany];
System.arraycopy(rescoredDocs.scoreDocs, 0, scoreDocs, 0, howMany);
rescoredDocs.scoreDocs = scoreDocs;
- return rescoredDocs;
}
+ return toRescoredDocs(rescoredDocs, docToOriginalScore);
Review Comment:
Don't we only want to do this if the user has asked for it?
##########
solr/core/src/test/org/apache/solr/search/TestReRankQParserPlugin.java:
##########
@@ -64,6 +64,80 @@ public void testReRankQParserPluginConstants() {
assertEquals(ReRankQParserPlugin.RERANK_OPERATOR, "reRankOperator");
}
+ @Test
+ public void testRerankReturnOriginalScore() throws Exception {
Review Comment:
This should be MatchScore not OriginalScore, right?
Also can we test returning the MatchScore for a non-reranked query? You mentioned that in the ref-guide update.
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -194,7 +193,6 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
private final StatsCache statsCache;
- private Set<String> metricNames = ConcurrentHashMap.newKeySet();
Review Comment:
Also this?
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -183,7 +182,7 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
@SuppressWarnings({"rawtypes"})
private final SolrCache[] cacheList;
- private DirectoryFactory directoryFactory;
+ private final DirectoryFactory directoryFactory;
Review Comment:
Why did you change this?
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "sijuv (via GitHub)" <gi...@apache.org>.
sijuv commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r1984122502
##########
solr/solr-ref-guide/modules/query-guide/pages/query-re-ranking.adoc:
##########
@@ -107,6 +107,9 @@ q=greetings&rq={!rerank reRankQuery=$rqq reRankDocs=1000 reRankWeight=3}&rqq=(hi
----
If a document matches the original query, but does not match the re-ranking query, the document's original score will remain.
+For reranked documents, an additional _matchScore_ field in the response will indicate the original score for a reranked doc. This
+is the score for the document prior to rerank being applied. For documents that were not reranked, the matchScore and score fields
+will have the same value.
Review Comment:
fixed
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2129734839
##########
solr/core/src/java/org/apache/solr/search/SolrReturnFields.java:
##########
@@ -50,6 +51,7 @@
public class SolrReturnFields extends ReturnFields {
// Special Field Keys
public static final String SCORE = "score";
+ public static final String MATCH_SCORE = "matchScore";
Review Comment:
That would at least unblock us for a release, so that we don't have to go and deprecate something later.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "sijuv (via GitHub)" <gi...@apache.org>.
sijuv commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r1984122197
##########
solr/core/src/java/org/apache/solr/search/ReRankCollector.java:
##########
@@ -205,18 +205,29 @@ public TopDocs topDocs(int start, int howMany) {
ScoreDoc[] scoreDocs = new ScoreDoc[howMany];
System.arraycopy(rescoredDocs.scoreDocs, 0, scoreDocs, 0, howMany);
rescoredDocs.scoreDocs = scoreDocs;
- return rescoredDocs;
}
+ return toRescoredDocs(rescoredDocs, docToOriginalScore);
Review Comment:
we dont have a handle to whether the `matchScore` field was requested at this point. Only when we get to ReturnFields do we have this info. Hence we need to have the logic of preserving the matchScore even if not requested.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "sijuv (via GitHub)" <gi...@apache.org>.
sijuv commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r1984120901
##########
solr/core/src/java/org/apache/solr/search/DocSlice.java:
##########
Review Comment:
done
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "sijuv (via GitHub)" <gi...@apache.org>.
sijuv commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r1983912923
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -194,7 +193,6 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
private final StatsCache statsCache;
- private Set<String> metricNames = ConcurrentHashMap.newKeySet();
Review Comment:
this is an unused field
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2129848607
##########
solr/core/src/java/org/apache/solr/search/SolrReturnFields.java:
##########
@@ -50,6 +51,7 @@
public class SolrReturnFields extends ReturnFields {
// Special Field Keys
public static final String SCORE = "score";
+ public static final String MATCH_SCORE = "matchScore";
Review Comment:
+1 with a TODO JIRA link
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2075879174
##########
solr/core/src/java/org/apache/solr/search/SolrReturnFields.java:
##########
@@ -50,6 +51,7 @@
public class SolrReturnFields extends ReturnFields {
// Special Field Keys
public static final String SCORE = "score";
+ public static final String MATCH_SCORE = "matchScore";
Review Comment:
I'm not loving another special case, vs a special function like say `matchScore()`. If we were starting Solr from scratch, even `score` would be `score()` IMO.
##########
solr/core/src/java/org/apache/solr/search/DocIterator.java:
##########
@@ -24,7 +24,7 @@
* <p>The order of the documents is determined by the context in which the DocIterator instance was
* retrieved.
*/
-public interface DocIterator extends Iterator<Integer> {
+public interface DocIterator extends Iterator<Integer>, DocIterationInfo {
Review Comment:
hmm; not sure this is really better. Maybe a hasScore was needed?
##########
solr/core/src/java/org/apache/solr/search/TopDocsSlice.java:
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search;
+
+import org.apache.lucene.search.TopDocs;
+import org.apache.lucene.search.TotalHits;
+
+/**
+ * <code>TopDocsSlice</code> implements DocList based off provided <code>TopDocs</code>.
+ *
+ * @since solr 9.9
+ */
+public class TopDocsSlice extends DocSlice {
Review Comment:
A very welcome addition; thanks
##########
solr/solrj/src/java/org/apache/solr/common/SolrDocument.java:
##########
@@ -195,6 +197,19 @@ public Collection<Object> getFieldValues(String name) {
return null;
}
+ /** Get the value or collection of values for a given field. */
+ public Map<String, Object> getSubsetOfFields(Set<String> fieldNames) {
Review Comment:
I'd suggest we be conservative about adding helper methods like this to major/popular Solr classes, without first identifying that it's widely useful.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman merged PR #3222:
URL: https://github.com/apache/solr/pull/3222
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2017531006
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -1984,18 +1982,22 @@ public ScoreMode scoreMode() {
populateNextCursorMarkFromTopDocs(qr, cmd, topDocs);
nDocsReturned = topDocs.scoreDocs.length;
- ids = new int[nDocsReturned];
- scores = needScores ? new float[nDocsReturned] : null;
- for (int i = 0; i < nDocsReturned; i++) {
- ScoreDoc scoreDoc = topDocs.scoreDocs[i];
- ids[i] = scoreDoc.doc;
- if (scores != null) scores[i] = scoreDoc.score;
+ int sliceLen = Math.min(lastDocRequested, nDocsReturned);
+ if (sliceLen < 0) {
+ sliceLen = 0;
+ }
+ if (!needScores) {
+ int[] ids = new int[nDocsReturned];
+ for (int i = 0; i < nDocsReturned; i++) {
+ ScoreDoc scoreDoc = topDocs.scoreDocs[i];
+ ids[i] = scoreDoc.doc;
+ }
+ docList = new DocSlice(0, sliceLen, ids, null, totalHits, maxScore, hitsRelation);
+ } else {
+ docList = new TopDocsSlice(0, sliceLen, topDocs, totalHits, maxScore, hitsRelation);
}
Review Comment:
Ok, I think this is the last question I have about this PR. Why not always use the `TopDocsSlice` here?
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678 Add support to return original score ( the query score ) in case of r… [solr]
Posted by "sijuv (via GitHub)" <gi...@apache.org>.
sijuv commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r1984125791
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -183,7 +182,7 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
@SuppressWarnings({"rawtypes"})
private final SolrCache[] cacheList;
- private DirectoryFactory directoryFactory;
+ private final DirectoryFactory directoryFactory;
Review Comment:
this variable cold be final.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #3222:
URL: https://github.com/apache/solr/pull/3222#issuecomment-2779569791
I'm planning on merging this early next week. Lots of tests to make sure everything works.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2124911514
##########
solr/core/src/java/org/apache/solr/search/SolrReturnFields.java:
##########
@@ -50,6 +51,7 @@
public class SolrReturnFields extends ReturnFields {
// Special Field Keys
public static final String SCORE = "score";
+ public static final String MATCH_SCORE = "matchScore";
Review Comment:
I agree, but at the time score() didn't exist (and still doesn't yet I guess). Happy to eventually move over to that, and deprecate this when it can be implemented well. (Discussion on https://github.com/apache/solr/pull/3340 probably makes more sense)
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on PR #3222:
URL: https://github.com/apache/solr/pull/3222#issuecomment-2937175581
> I wish I reviewed this earlier... feel free to tag me on geeky Solr internals... there are only a few of us that know this stuff
Sorry have been vacationing and busy. Happy to make changes before 9.9 is released. There was a lot to consider here, so had to make a few design decisions, but happy to revisit them if you have other ways you want to go.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17678, SOLR-17732: Add matchScore support for ReRank queries [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #3222:
URL: https://github.com/apache/solr/pull/3222#discussion_r2136046774
##########
solr/core/src/java/org/apache/solr/search/SolrReturnFields.java:
##########
@@ -50,6 +51,7 @@
public class SolrReturnFields extends ReturnFields {
// Special Field Keys
public static final String SCORE = "score";
+ public static final String MATCH_SCORE = "matchScore";
Review Comment:
[SOLR-17784](https://issues.apache.org/jira/browse/SOLR-17784). I'll go ahead and make a PR to add the parentheses.
--
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@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org