tomglk commented on a change in pull request #123:
URL: https://github.com/apache/solr/pull/123#discussion_r633270956



##########
File path: 
solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldValueFeature.java
##########
@@ -106,18 +106,29 @@ public FieldValueFeatureWeight(IndexSearcher searcher,
 
     /**
      * 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()) {
-        return new DocValuesFieldValueFeatureScorer(this, context,
-            DocIdSetIterator.all(DocIdSetIterator.NO_MORE_DOCS), 
schemaField.getType());
+
+        FieldInfo fieldInfo = 
context.reader().getFieldInfos().fieldInfo(field);
+        DocValuesType docValuesType = fieldInfo != null ? 
fieldInfo.getDocValuesType() : DocValuesType.NONE;
+
+        if (DocValuesType.NUMERIC.equals(docValuesType) || 
DocValuesType.SORTED.equals(docValuesType)) {
+          return new DocValuesFieldValueFeatureScorer(this, context,
+                  DocIdSetIterator.all(DocIdSetIterator.NO_MORE_DOCS), 
schemaField.getType(), docValuesType);
+          // If type is NONE, this segment has no docs with this field. That's 
not a problem, because we won't call score() anyway

Review comment:
       Great observation! I completely overlooked this.
   Your reasoning makes complete sense to me and some debugging verified the 
behavior you described.
   
   Even if it may be an edge case, I think it makes sense to make it explicit.
   In terms of performance you are also right. Trying to read the stored fields 
before returning the default value surely is more costly than just returning 
the value.
   
   I started to create a `DefaultValueFieldValueFeatureScorer` that always 
returns the default value, but until now I am struggling to find a way to test 
this.
   
   I hope your bike is well now! :)




-- 
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

Reply via email to