cpoerschke commented on a change in pull request #166:
URL: https://github.com/apache/solr/pull/166#discussion_r656440736
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRScoringQuery.java
##########
@@ -223,6 +223,7 @@ public ModelWeight createWeight(IndexSearcher searcher,
ScoreMode scoreMode, flo
else{
createWeightsParallel(searcher, scoreMode.needsScores(), featureWeights,
features);
}
+ // TODO: potential prefetchFields computation site?
Review comment:
> ... I never heard about `Clonable`, but that sounds like we could use
it here. :)
I've seen it but never used it myself as yet, so learnt something new here
too :)
> Yes, it makes the code more comprehensive. But my problem is, that now we
just throw a generic `UnsupportedOperationException` instead of providing more
information about the cause like we did with the custom message.
Yes, it's a bit of a trade-off i.e. more concise code in the general common
code path case at the expense of only a generic exception in the
rare/impossible code path when an exception is thrown. From my current
understanding of the code the
```
throw new UnsupportedOperationException("Feature " + name + " is in use by a
model. Its prefetchingFields may not be changed!");
```
can't ever happen, right? And so if it did happen then some very strange bug
or problem presumably would have arisen and whilst the name of the feature
might be useful to figure out the details perhaps the "is in use by a model"
claim made by the exception would not be accurate in that scenario i.e. when
the impossible happens very few assumptions about the state of the systems can
then be made.
Removal of the `keepFeaturesFinalAfterModelCreation` boolean also means that
we don't need to worry about boolean vs. AtomicBoolean or other thread-safety
for it and it being or not being included when the object is cloned.
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
