risdenk commented on code in PR #1557:
URL: https://github.com/apache/solr/pull/1557#discussion_r1163181705
##########
solr/core/src/test/org/apache/solr/uninverting/TestFieldCacheVsDocValues.java:
##########
@@ -175,7 +175,7 @@ public void testHugeBinaryValues() throws Exception {
BinaryDocValues s = FieldCache.DEFAULT.getTerms(ar, "field");
for (int docID = 0; docID < docBytes.size(); docID++) {
- Document doc = ar.document(docID);
+ Document doc = ar.storedFields().document(docID);
Review Comment:
`ar.storedFields()` can move outside the for loop
##########
solr/core/src/test/org/apache/solr/search/function/TestOrdValues.java:
##########
@@ -139,7 +139,7 @@ private void doTestExactScore(String field, boolean
inOrder) throws Exception {
ScoreDoc sd[] = td.scoreDocs;
for (int i = 0; i < sd.length; i++) {
float score = sd[i].score;
- String id = s.getIndexReader().document(sd[i].doc).get(ID_FIELD);
+ String id =
s.getIndexReader().storedFields().document(sd[i].doc).get(ID_FIELD);
Review Comment:
`s.getIndexReader().storedFields()` can be moved outside for for loop
##########
solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java:
##########
@@ -372,7 +372,7 @@ public void doc(int docId, StoredFieldVisitor visitor)
throws IOException {
Document cached = doc(docId);
visitFromCached(cached, visitor);
} else {
- searcher.getIndexReader().document(docId, visitor);
+ searcher.getIndexReader().storedFields().document(docId, visitor);
Review Comment:
I wonder if `searcher.getIndexReader().storedFields()` can be constructed in
the constructor? the searcher is stored there. This would avoid recreating in a
few places in this file.
##########
solr/modules/ltr/src/java/org/apache/solr/ltr/feature/FieldValueFeature.java:
##########
@@ -177,7 +177,8 @@ public FieldValueFeatureScorer(
public float score() throws IOException {
try {
- final Document document = context.reader().document(itr.docID(),
fieldAsSet);
+ final Document document =
+ context.reader().storedFields().document(itr.docID(),
fieldAsSet);
Review Comment:
`context.reader().storedFields()` - I'm not so sure about this case - but
can this be constructed in the constructor? Is there a need to do this for
every document score? It looks like context.reader() would exist in the
constructor. I don't know if there are downsides to this.
##########
solr/core/src/java/org/apache/solr/highlight/DefaultSolrHighlighter.java:
##########
@@ -695,7 +695,7 @@ protected Object doHighlightingByHighlighter(
// Try term vectors, which is faster
// note: offsets are minimally sufficient for this HL.
- final Fields tvFields = schemaField.storeTermOffsets() ?
reader.getTermVectors(docId) : null;
+ final Fields tvFields = schemaField.storeTermOffsets() ?
reader.termVectors().get(docId) : null;
Review Comment:
I haven't tracked down this change - but reader is passed in - maybe it
makes sense to just pass in `reader.termVectors()` instead?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
