dsmiley commented on code in PR #1557:
URL: https://github.com/apache/solr/pull/1557#discussion_r1545466879
##########
solr/core/src/java/org/apache/solr/search/SolrDocumentFetcher.java:
##########
@@ -114,9 +114,33 @@ public class SolrDocumentFetcher {
private final Set<String> largeFields;
- private Collection<String> storedHighlightFieldNames; // lazy populated; use
getter
+ @SuppressWarnings({"unchecked", "rawtypes"})
+ private final Collection<String>[] storedHighlightFieldNames =
+ new Collection[1]; // lazy populated; use getter
+
+ @SuppressWarnings({"unchecked", "rawtypes"})
+ private final Collection<String>[] indexedFieldNames =
+ new Collection[1]; // lazy populated; use getter
Review Comment:
Since you create the array indirection here, this means the copied
SolrDocumentFetcher will get a new indirection instead of the template's
indirection. Thus what's computed will be repeated, not inherited.
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -427,7 +429,16 @@ public SolrIndexSearcher(
}
public SolrDocumentFetcher getDocFetcher() {
- return docFetcher;
+ try {
+ return docFetcher.createInstance();
+ } catch (IOException e) {
+ // TODO: should we instead throw IOException?
+ throw new RuntimeIOException(e);
Review Comment:
Let's not use a Jetty class where it doesn't belong
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -427,7 +429,16 @@ public SolrIndexSearcher(
}
public SolrDocumentFetcher getDocFetcher() {
- return docFetcher;
+ try {
+ return docFetcher.createInstance();
+ } catch (IOException e) {
+ // TODO: should we instead throw IOException?
+ throw new RuntimeIOException(e);
+ }
+ }
+
+ public <T> T interrogateDocFetcher(Function<SolrDocumentFetcher, T> func) {
Review Comment:
needs javadoc; it's weird looking
##########
solr/core/src/java/org/apache/solr/highlight/DefaultSolrHighlighter.java:
##########
@@ -482,15 +483,28 @@ public NamedList<Object> doHighlighting(
// Lazy container for fvh and fieldQuery
FvhContainer fvhContainer = new FvhContainer(null, null);
- IndexReader reader =
- new
TermVectorReusingLeafReader(req.getSearcher().getSlowAtomicReader()); //
SOLR-5855
+ IndexReader reader = req.getSearcher().getSlowAtomicReader();
+ IOSupplier<TermVectors> tvSupplier =
Review Comment:
Just a suggestion: A simplification I see is to put this lazy load aspect
directly into ReusingTermVectors. Then we can pass TermVectors (specifically
of type ReusingTermVectors) and not need IOSupplier.
--
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]
