cpoerschke commented on a change in pull request #123:
URL: https://github.com/apache/solr/pull/123#discussion_r633701951
##########
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:
> ... to find a way to test this. ...
Thinking conceptually (without actually trying it) one way could be like
this ...
* imagine `TestFieldValueFeature` had a static inner class
`FooBarFieldValueFeature extends FieldValueFeature` and for some of its tests
it uses that instead of the 'real' `FieldValueFeature` class
* imagine `FooBarFieldValueFeature` can observe whatever needs to be
observed and capture the observations in class level variables
* if the class level variables are public or package scope the test has
access to them
Well, that's the theory but in practice what we wish to observe and test is
not directly in `FieldValueFeature` but inside the `FieldValueFeatureWeight`
created, but still it could be possible e.g. something like
```
class FooBarFieldValueFeature extends FieldValueFeature {
static AtomicBoolean seenFieldValueFeatureScorer = false; // could also be
counter instead of boolean
@Override
public FeatureWeight createWeight(...) throws IOException {
return new FooBarFieldValueFeatureWeight(...);
}
public class FooBarFieldValueFeatureWeight extends FieldValueFeatureWeight
{
@Override
public FeatureScorer scorer(LeafReaderContext context) throws
IOException {
FeatureScorer featureScorer = super.scorer();
if (featureScorer instanceof FieldValueFeatureScorer) {
seenFieldValueFeatureScorer = true;
}
return featureScorer;
}
}
}
```
structurally and then in the test
`assertFalse(FooBarFieldValueFeature.seenFieldValueFeatureScorer);`
Does that kind of convey the idea? Essentially the idea is to extend the
"real" class with a "test" class, for the test class to delegate to the real
class and for the test class to be "shaped" to suit the test's purposes.
--
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
