cpoerschke commented on a change in pull request #123:
URL: https://github.com/apache/solr/pull/123#discussion_r637903714
##########
File path:
solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldValueFeature.java
##########
@@ -83,18 +104,52 @@ public FeatureWeight createWeight(IndexSearcher searcher,
boolean needsScores,
}
public class FieldValueFeatureWeight extends FeatureWeight {
+ private final SchemaField schemaField;
public FieldValueFeatureWeight(IndexSearcher searcher,
SolrQueryRequest request, Query originalQuery, Map<String,String[]>
efi) {
super(FieldValueFeature.this, searcher, request, originalQuery, efi);
+ if (searcher instanceof SolrIndexSearcher) {
+ schemaField = ((SolrIndexSearcher)
searcher).getSchema().getFieldOrNull(field);
+ } else { // some tests pass a null or a non-SolrIndexSearcher searcher
+ schemaField = null;
+ }
}
+ /**
+ * Return a FeatureScorer that uses docValues or storedFields if no
docValues are present
+ *
+ * @param context the segment this FeatureScorer is working with
+ * @return FeatureScorer for the current segment and field
+ * @throws IOException as defined by abstract class Feature
+ */
@Override
public FeatureScorer scorer(LeafReaderContext context) throws IOException {
+ if (schemaField != null && !schemaField.stored() &&
schemaField.hasDocValues()) {
+
+ final FieldInfo fieldInfo =
context.reader().getFieldInfos().fieldInfo(field);
+ final DocValuesType docValuesType = fieldInfo != null ?
fieldInfo.getDocValuesType() : DocValuesType.NONE;
+
+ if (DocValuesType.NUMERIC.equals(docValuesType)) {
+ return new NumericDocValuesFieldValueFeatureScorer(this, context,
+ DocIdSetIterator.all(DocIdSetIterator.NO_MORE_DOCS),
schemaField.getType().getNumberType());
+ } else if (DocValuesType.SORTED.equals(docValuesType)) {
+ return new SortedDocValuesFieldValueFeatureScorer(this, context,
+ DocIdSetIterator.all(DocIdSetIterator.NO_MORE_DOCS));
+ } else if (DocValuesType.NONE.equals(docValuesType)) {
+ // Using a fallback feature scorer because this segment has no
documents with a doc value for the current field
+ return new DefaultValueFieldValueFeatureScorer(this,
DocIdSetIterator.all(DocIdSetIterator.NO_MORE_DOCS));
+ }
Review comment:
2/2 ... combined with here diverting any unexpected `getNumberType()`
enums onto the "is not supported" code path? Although having said that, I don't
know what the existing `FieldValueFeatureScorer` would return for a stored
`DATE` field i.e. perhaps back compat considerations apply, hmm.
##########
File path:
solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldValueFeature.java
##########
@@ -146,5 +201,137 @@ public float getMaxScore(int upTo) throws IOException {
return Float.POSITIVE_INFINITY;
}
}
+
+ /**
+ * A FeatureScorer that reads the numeric docValues for a field
+ */
+ public final class NumericDocValuesFieldValueFeatureScorer extends
FeatureScorer {
+ private final NumericDocValues docValues;
+ private final NumberType numberType;
+
+ public NumericDocValuesFieldValueFeatureScorer(final FeatureWeight
weight, final LeafReaderContext context,
+ final DocIdSetIterator itr,
final NumberType numberType) {
+ super(weight, itr);
+ this.numberType = numberType;
+
+ NumericDocValues docValues;
+ try {
+ docValues = DocValues.getNumeric(context.reader(), field);
+ } catch (IOException e) {
+ throw new IllegalArgumentException("Could not read numeric docValues
for field " + field);
+ }
+ this.docValues = docValues;
+ }
+
+ @Override
+ public float score() throws IOException {
+ if (docValues.advanceExact(itr.docID())) {
+ return readNumericDocValues();
+ }
+ return FieldValueFeature.this.getDefaultValue();
+ }
+
+ /**
+ * Read the numeric value for a field and convert the different number
types to float.
+ *
+ * @return The numeric value that the docValues contain for the current
document
+ * @throws IOException if docValues cannot be read
+ */
+ private float readNumericDocValues() throws IOException {
+ if (NumberType.FLOAT.equals(numberType)) {
+ // convert float value that was stored as long back to float
+ return Float.intBitsToFloat((int) docValues.longValue());
+ } else if (NumberType.DOUBLE.equals(numberType)) {
+ // handle double value conversion
+ return (float) Double.longBitsToDouble(docValues.longValue());
+ }
+ // just take the long value
+ return docValues.longValue();
+ }
Review comment:
1/2 So I was trying to learn and understand better w.r.t. why the
`intBitsToFloat` and `longBitsToDouble` are the conversions to use here and
that led to
https://github.com/apache/lucene-solr/blob/releases/lucene-solr/8.8.2/solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java#L603-L644
and the realisation that `DATE` is a `numberType` possibility!
Would you have any thoughts on using a `switch` statement here with
FLOAT/DOUBLE/INT/LONG all valid and to throw an exception for anything else ...
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
