New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fold InternalSearchHits and friends into their interfaces #23042
Conversation
We have a bunch of interfaces that have only a single implementation for 6 years now. These interfaces are pretty useless from a SW development perspective and only add unnecessary abstractions. They also require lots of casting in many places where we expect that there is only one concrete implementation. This change removes the interfaces, makes all of the classes final and removes the duplicate `foo` `getFoo` accessors in favor of `getFoo` from these classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
for (int i = 0; i < reducedTopDocs.scoreDocs.length; i++) { | ||
ScoreDoc scoreDoc = reducedTopDocs.scoreDocs[i]; | ||
int position; | ||
do { | ||
position = tracker[scoreDoc.shardIndex]++; | ||
} while (shardDocs[scoreDoc.shardIndex].scoreDocs[position] != scoreDoc); | ||
hits[i] = (InternalSearchHit) shardHits[scoreDoc.shardIndex].getAt(position); | ||
hits[i] = (SearchHit) shardHits[scoreDoc.shardIndex].getAt(position); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the cast necessary?
@@ -56,7 +55,7 @@ public void accept(SearchRequest searchRequest, SearchResponse searchResponse) { | |||
return ; | |||
} | |||
for (SearchHit hit : searchResponse.getHits()) { | |||
InternalSearchHit internalHit = (InternalSearchHit) hit; | |||
SearchHit internalHit = (SearchHit) hit; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessary cast
@@ -133,14 +133,14 @@ private Object randomOfType(SortField.Type type) { | |||
|
|||
@Override | |||
protected void assertReduced(InternalTopHits reduced, List<InternalTopHits> inputs) { | |||
InternalSearchHits actualHits = (InternalSearchHits) reduced.getHits(); | |||
List<Tuple<ScoreDoc, InternalSearchHit>> allHits = new ArrayList<>(); | |||
SearchHits actualHits = (SearchHits) reduced.getHits(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the cast necessary?
InternalSearchHits internalHits = (InternalSearchHits) inputs.get(input).getHits(); | ||
totalHits += internalHits.totalHits(); | ||
maxScore = max(maxScore, internalHits.maxScore()); | ||
SearchHits internalHits = (SearchHits) inputs.get(input).getHits(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here?
We have a bunch of interfaces that have only a single implementation for 6 years now. These interfaces are pretty useless from a SW development perspective and only add unnecessary abstractions. They also require lots of casting in many places where we expect that there is only one concrete implementation. This change removes the interfaces, makes all of the classes final and removes the duplicate `foo` `getFoo` accessors in favor of `getFoo` from these classes. This is a backport of ecb01c1 that deprecates all methods that are removed in 6.0
We have a bunch of interfaces that have only a single implementation
for 6 years now. These interfaces are pretty useless from a SW development
perspective and only add unnecessary abstractions. They also require
lots of casting in many places where we expect that there is only one
concrete implementation. This change removes the interfaces, makes
all of the classes final and removes the duplicate
foo
getFoo
accessorsin favor of
getFoo
from these classes.