-
Notifications
You must be signed in to change notification settings - Fork 739
[SOLR-15030] Add score() function that returns the score of the current hit #3340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e3432d9
c070dd1
4e8f9f2
190d8ea
1377ffb
11f8013
d99c97d
db435dc
d85b077
10d797a
f461da7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,7 +59,7 @@ public class SolrReturnFields extends ReturnFields { | |
|
||
// Field names that are OK to include in the response. | ||
// This will include pseudo fields, lucene fields, and matching globs | ||
private Set<String> okFieldNames = new HashSet<>(); | ||
private final Set<String> okFieldNames = new HashSet<>(); | ||
|
||
// The list of explicitly requested fields | ||
// Order is important for CSVResponseWriter | ||
|
@@ -515,6 +515,12 @@ private void addField( | |
String disp = (key == null) ? field : key; | ||
augmenters.addTransformer(new ScoreAugmenter(disp)); | ||
scoreDependentFields.put(disp, disp.equals(SCORE) ? "" : SCORE); | ||
} else if (key != null && isPseudoField) { | ||
// SOLR-15030: a pseudo-field based on the function query may need scores, | ||
// so we consider all pseudo-fields as potentially requiring scores. | ||
// At the same time, we don't set _wantScore = true because the field | ||
// list must explicitly include 'score' field to enable scores | ||
Comment on lines
+521
to
+522
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused or challenge what you say in this sentence. A DocTransformer could want the score. Granted, a DocTransformer doesn't have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nor does a Addressing that is perhaps for another issue/PR. Progress not perfection. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume that the work-around to Lucene standardizes on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I thought was that for the query with |
||
scoreDependentFields.put(key, ""); | ||
} | ||
} | ||
|
||
|
@@ -576,7 +582,7 @@ public boolean wantsScore() { | |
|
||
@Override | ||
public Map<String, String> getScoreDependentReturnFields() { | ||
return scoreDependentFields; | ||
return _wantsScore ? scoreDependentFields : Map.of(); | ||
} | ||
|
||
@Override | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
/* | ||
* 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.function; | ||
|
||
import java.io.IOException; | ||
import java.util.Map; | ||
import org.apache.lucene.index.LeafReaderContext; | ||
import org.apache.lucene.queries.function.FunctionValues; | ||
import org.apache.lucene.queries.function.ValueSource; | ||
import org.apache.lucene.queries.function.docvalues.FloatDocValues; | ||
import org.apache.lucene.search.Scorable; | ||
import org.apache.solr.common.SolrException; | ||
|
||
/** Returns the score of the current hit. */ | ||
public final class ScoreFunction extends ValueSource { | ||
|
||
public static final ScoreFunction INSTANCE = new ScoreFunction(); | ||
|
||
@Override | ||
public FunctionValues getValues(Map<Object, Object> context, LeafReaderContext readerContext) | ||
throws IOException { | ||
Scorable scorer = (Scorable) context.get("scorer"); | ||
if (scorer == null) { | ||
throw new SolrException( | ||
SolrException.ErrorCode.BAD_REQUEST, "score function cannot access the document scores"); | ||
} | ||
|
||
return new FloatDocValues(this) { | ||
@Override | ||
public float floatVal(int doc) throws IOException { | ||
assert scorer.docID() == doc | ||
: "Expected scorer to be positioned on doc: " + doc + ", but was: " + scorer.docID(); | ||
return scorer.score(); | ||
} | ||
|
||
@Override | ||
public boolean exists(int doc) { | ||
assert scorer.docID() == doc | ||
: "Expected scorer to be positioned on doc: " + doc + ", but was: " + scorer.docID(); | ||
return true; | ||
} | ||
}; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
return o instanceof ScoreFunction; | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return getClass().hashCode(); | ||
} | ||
|
||
@Override | ||
public String description() { | ||
return "score"; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are three other
ScoreAndDoc
helper classes (with identical implementation) scattered throughout the codebase. Maybe it's worth consolidating into one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One top level class makes sense!