Skip to content
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

Make use of constants and utility methods to build empty SearchHits instances #103983

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
package org.elasticsearch.plugin.noop.action.search;

import org.apache.lucene.search.TotalHits;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
Expand All @@ -18,7 +17,6 @@
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.plugin.noop.NoopPlugin;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.aggregations.InternalAggregations;
import org.elasticsearch.search.profile.SearchProfileResults;
Expand All @@ -44,7 +42,7 @@ public TransportNoopSearchAction(TransportService transportService, ActionFilter
protected void doExecute(Task task, SearchRequest request, ActionListener<SearchResponse> listener) {
listener.onResponse(
new SearchResponse(
new SearchHits(new SearchHit[0], new TotalHits(0L, TotalHits.Relation.EQUAL_TO), 0.0f),
SearchHits.EMPTY_WITH_TOTAL_HITS,
InternalAggregations.EMPTY,
new Suggest(Collections.emptyList()),
false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.SearchShardTarget;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xcontent.ToXContent;
Expand Down Expand Up @@ -208,15 +209,14 @@ public void testNoResults() throws Exception {
}
}
}
SearchHit[] hits = new SearchHit[0];
DiscountedCumulativeGain dcg = new DiscountedCumulativeGain();
EvalQueryQuality result = dcg.evaluate("id", hits, ratedDocs);
EvalQueryQuality result = dcg.evaluate("id", SearchHits.EMPTY, ratedDocs);
assertEquals(0.0d, result.metricScore(), DELTA);
assertEquals(0, filterUnratedDocuments(result.getHitsAndRatings()).size());

// also check normalized
dcg = new DiscountedCumulativeGain(true, null, 10);
result = dcg.evaluate("id", hits, ratedDocs);
result = dcg.evaluate("id", SearchHits.EMPTY, ratedDocs);
assertEquals(0.0d, result.metricScore(), DELTA);
assertEquals(0, filterUnratedDocuments(result.getHitsAndRatings()).size());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.SearchShardTarget;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xcontent.ToXContent;
Expand Down Expand Up @@ -114,7 +115,7 @@ private SearchHit[] createSearchHits(List<RatedDocument> rated, Integer[] releva
*/
public void testNoResults() throws Exception {
ExpectedReciprocalRank err = new ExpectedReciprocalRank(5, 0, 10);
assertEquals(0.0, err.evaluate("id", new SearchHit[0], Collections.emptyList()).metricScore(), DELTA);
assertEquals(0.0, err.evaluate("id", SearchHits.EMPTY, Collections.emptyList()).metricScore(), DELTA);
}

public void testParseFromXContent() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.SearchShardTarget;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xcontent.ToXContent;
Expand Down Expand Up @@ -150,8 +151,7 @@ public void testEvaluationNoRelevantInResults() {
}

public void testNoResults() throws Exception {
SearchHit[] hits = new SearchHit[0];
EvalQueryQuality evaluated = (new MeanReciprocalRank()).evaluate("id", hits, Collections.emptyList());
EvalQueryQuality evaluated = (new MeanReciprocalRank()).evaluate("id", SearchHits.EMPTY, Collections.emptyList());
assertEquals(0.0d, evaluated.metricScore(), 0.00001);
assertEquals(-1, ((MeanReciprocalRank.Detail) evaluated.getMetricDetails()).getFirstRelevantRank());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.SearchShardTarget;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xcontent.ToXContent;
Expand Down Expand Up @@ -136,8 +137,7 @@ public void testNoRatedDocs() throws Exception {
}

public void testNoResults() throws Exception {
SearchHit[] hits = new SearchHit[0];
EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", hits, Collections.emptyList());
EvalQueryQuality evaluated = (new PrecisionAtK()).evaluate("id", SearchHits.EMPTY, Collections.emptyList());
assertEquals(0.0d, evaluated.metricScore(), 0.00001);
assertEquals(0, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved());
assertEquals(0, ((PrecisionAtK.Detail) evaluated.getMetricDetails()).getRetrieved());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.SearchShardTarget;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xcontent.ToXContent;
Expand Down Expand Up @@ -113,7 +114,7 @@ public void testNoRatedDocs() throws Exception {
}

public void testNoResults() throws Exception {
EvalQueryQuality evaluated = (new RecallAtK()).evaluate("id", new SearchHit[0], Collections.emptyList());
EvalQueryQuality evaluated = (new RecallAtK()).evaluate("id", SearchHits.EMPTY, Collections.emptyList());
assertEquals(0.0d, evaluated.metricScore(), 0.00001);
assertEquals(0, ((RecallAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved());
assertEquals(0, ((RecallAtK.Detail) evaluated.getMetricDetails()).getRelevant());
Expand All @@ -123,7 +124,7 @@ public void testNoResultsWithRatedDocs() throws Exception {
List<RatedDocument> rated = new ArrayList<>();
rated.add(createRatedDoc("test", "0", RELEVANT_RATING));

EvalQueryQuality evaluated = (new RecallAtK()).evaluate("id", new SearchHit[0], rated);
EvalQueryQuality evaluated = (new RecallAtK()).evaluate("id", SearchHits.EMPTY, rated);
assertEquals(0.0d, evaluated.metricScore(), 0.00001);
assertEquals(0, ((RecallAtK.Detail) evaluated.getMetricDetails()).getRelevantRetrieved());
assertEquals(1, ((RecallAtK.Detail) evaluated.getMetricDetails()).getRelevant());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ private static MockTransportService startTransport(
SearchRequest::new,
(request, channel, task) -> channel.sendResponse(
new SearchResponse(
new SearchHits(new SearchHit[0], new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
SearchHits.empty(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
InternalAggregations.EMPTY,
null,
false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.rest.action.RestActions;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.aggregations.Aggregations;
import org.elasticsearch.search.aggregations.InternalAggregations;
Expand Down Expand Up @@ -1368,9 +1367,8 @@ public String toString() {

// public for tests
public static SearchResponse empty(Supplier<Long> tookInMillisSupplier, Clusters clusters) {
SearchHits searchHits = new SearchHits(new SearchHit[0], new TotalHits(0L, TotalHits.Relation.EQUAL_TO), Float.NaN);
return new SearchResponse(
searchHits,
SearchHits.empty(new TotalHits(0L, TotalHits.Relation.EQUAL_TO), Float.NaN),
InternalAggregations.EMPTY,
null,
false,
Expand Down
10 changes: 7 additions & 3 deletions server/src/main/java/org/elasticsearch/search/SearchHits.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
public final class SearchHits implements Writeable, ChunkedToXContent, Iterable<SearchHit> {

public static final SearchHit[] EMPTY = new SearchHit[0];
public static final SearchHits EMPTY_WITH_TOTAL_HITS = new SearchHits(EMPTY, new TotalHits(0, Relation.EQUAL_TO), 0);
public static final SearchHits EMPTY_WITHOUT_TOTAL_HITS = new SearchHits(EMPTY, null, 0);
public static final SearchHits EMPTY_WITH_TOTAL_HITS = SearchHits.empty(new TotalHits(0, Relation.EQUAL_TO), 0);
public static final SearchHits EMPTY_WITHOUT_TOTAL_HITS = SearchHits.empty(null, 0);

private final SearchHit[] hits;
private final TotalHits totalHits;
Expand All @@ -48,6 +48,10 @@ public final class SearchHits implements Writeable, ChunkedToXContent, Iterable<
@Nullable
private final Object[] collapseValues;

public static SearchHits empty(@Nullable TotalHits totalHits, float maxScore) {
return new SearchHits(EMPTY, totalHits, maxScore);
}

public SearchHits(SearchHit[] hits, @Nullable TotalHits totalHits, float maxScore) {
this(hits, totalHits, maxScore, null, null, null);
}
Expand Down Expand Up @@ -235,7 +239,7 @@ public static SearchHits fromXContent(XContentParser parser) throws IOException
}
}
}
return new SearchHits(hits.toArray(new SearchHit[0]), totalHits, maxScore);
return new SearchHits(hits.toArray(SearchHits.EMPTY), totalHits, maxScore);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ public void execute(SearchContext context, int[] docIdsToLoad) {

if (docIdsToLoad == null || docIdsToLoad.length == 0) {
// no individual hits to process, so we shortcut
SearchHits hits = new SearchHits(new SearchHit[0], context.queryResult().getTotalHits(), context.queryResult().getMaxScore());
context.fetchResult().shardResult(hits, null);
context.fetchResult()
.shardResult(SearchHits.empty(context.queryResult().getTotalHits(), context.queryResult().getMaxScore()), null);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ void sendExecuteMultiSearch(MultiSearchRequest request, SearchTask task, ActionL
)
);

SearchHits hits = new SearchHits(new SearchHit[0], new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f);
SearchHits hits = SearchHits.empty(new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f);
ExpandSearchPhase phase = new ExpandSearchPhase(mockSearchPhaseContext, hits, () -> new SearchPhase("test") {
@Override
public void run() {
Expand Down Expand Up @@ -274,7 +274,7 @@ void sendExecuteMultiSearch(MultiSearchRequest request, SearchTask task, ActionL
.preference("foobar")
.routing("baz");

SearchHits hits = new SearchHits(new SearchHit[0], new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f);
SearchHits hits = SearchHits.empty(new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f);
ExpandSearchPhase phase = new ExpandSearchPhase(mockSearchPhaseContext, hits, () -> new SearchPhase("test") {
@Override
public void run() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ void sendExecuteMultiSearch(
fields.forEach((f, values) -> hit.setDocumentField(f, new DocumentField(f, values, List.of())));
searchHits = new SearchHits(new SearchHit[] { hit }, new TotalHits(1, TotalHits.Relation.EQUAL_TO), 1.0f);
} else {
searchHits = new SearchHits(new SearchHit[0], new TotalHits(0, TotalHits.Relation.EQUAL_TO), 1.0f);
searchHits = SearchHits.empty(new TotalHits(0, TotalHits.Relation.EQUAL_TO), 1.0f);
}
responses[i] = new MultiSearchResponse.Item(
new SearchResponse(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,7 @@ private static AtomicArray<SearchPhaseResult> generateFetchResults(
}
}
}
SearchHit[] hits = searchHits.toArray(new SearchHit[0]);
SearchHit[] hits = searchHits.toArray(SearchHits.EMPTY);
ProfileResult profileResult = profile && searchHits.size() > 0
? new ProfileResult("fetch", "fetch", Map.of(), Map.of(), randomNonNegativeLong(), List.of())
: null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,8 @@ public void testMergeProfileResults() throws InterruptedException {
for (int i = 0; i < numResponses; i++) {
SearchProfileResults profile = SearchProfileResultsTests.createTestItem();
expectedProfile.putAll(profile.getShardResults());
SearchHits searchHits = new SearchHits(new SearchHit[0], new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN);
SearchResponse searchResponse = new SearchResponse(
searchHits,
SearchHits.empty(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
null,
null,
false,
Expand Down Expand Up @@ -408,7 +407,7 @@ public void testMergeCompletionSuggestions() throws InterruptedException {
completionSuggestion.addTerm(options);
suggestions.add(completionSuggestion);
Suggest suggest = new Suggest(suggestions);
SearchHits searchHits = new SearchHits(new SearchHit[0], null, Float.NaN);
SearchHits searchHits = SearchHits.empty(null, Float.NaN);
SearchResponse searchResponse = new SearchResponse(
searchHits,
null,
Expand Down Expand Up @@ -494,9 +493,8 @@ public void testMergeCompletionSuggestionsTieBreak() throws InterruptedException
completionSuggestion.addTerm(options);
suggestions.add(completionSuggestion);
Suggest suggest = new Suggest(suggestions);
SearchHits searchHits = new SearchHits(new SearchHit[0], null, Float.NaN);
SearchResponse searchResponse = new SearchResponse(
searchHits,
SearchHits.empty(null, Float.NaN),
null,
suggest,
false,
Expand Down Expand Up @@ -565,7 +563,6 @@ public void testMergeEmptyFormat() throws InterruptedException {
Collections.emptyMap()
);

SearchHits searchHits = new SearchHits(new SearchHit[0], null, Float.NaN);
try (
SearchResponseMerger searchResponseMerger = new SearchResponseMerger(
0,
Expand All @@ -578,7 +575,7 @@ public void testMergeEmptyFormat() throws InterruptedException {
for (Max max : Arrays.asList(max1, max2)) {
InternalAggregations aggs = InternalAggregations.from(Arrays.asList(max));
SearchResponse searchResponse = new SearchResponse(
searchHits,
SearchHits.empty(null, Float.NaN),
aggs,
null,
false,
Expand Down Expand Up @@ -645,9 +642,8 @@ public void testMergeAggs() throws InterruptedException {
);
InternalDateRange range = factory.create(rangeAggName, singletonList(bucket), DocValueFormat.RAW, false, emptyMap());
InternalAggregations aggs = InternalAggregations.from(Arrays.asList(range, max));
SearchHits searchHits = new SearchHits(new SearchHit[0], null, Float.NaN);
SearchResponse searchResponse = new SearchResponse(
searchHits,
SearchHits.empty(null, Float.NaN),
aggs,
null,
false,
Expand Down Expand Up @@ -977,16 +973,8 @@ public void testMergeEmptySearchHitsWithNonEmpty() {
}
}
{
SearchHits empty = new SearchHits(
new SearchHit[0],
new TotalHits(0, TotalHits.Relation.EQUAL_TO),
Float.NaN,
null,
null,
null
);
SearchResponse searchResponse = new SearchResponse(
empty,
SearchHits.empty(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
null,
null,
false,
Expand Down Expand Up @@ -1041,9 +1029,8 @@ public void testMergeOnlyEmptyHits() {
long previousValue = expectedTotalHits == null ? 0 : expectedTotalHits.value;
expectedTotalHits = new TotalHits(Math.min(previousValue + totalHits.value, trackTotalHitsUpTo), totalHitsRelation);
}
SearchHits empty = new SearchHits(new SearchHit[0], totalHits, Float.NaN, null, null, null);
SearchResponse searchResponse = new SearchResponse(
empty,
SearchHits.empty(totalHits, Float.NaN),
null,
null,
false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.search.DummyQueryBuilder;
import org.elasticsearch.search.Scroll;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.SearchService;
import org.elasticsearch.search.SearchShardTarget;
Expand Down Expand Up @@ -479,26 +478,6 @@ private MockTransportService[] startTransport(
return mockTransportServices;
}

private static SearchResponse emptySearchResponse() {
return new SearchResponse(
new SearchHits(new SearchHit[0], new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
InternalAggregations.EMPTY,
null,
false,
null,
null,
1,
null,
1,
1,
0,
100,
ShardSearchFailure.EMPTY_ARRAY,
SearchResponse.Clusters.EMPTY,
null
);
}

public void testCCSRemoteReduceMergeFails() throws Exception {
int numClusters = randomIntBetween(2, 10);
DiscoveryNode[] nodes = new DiscoveryNode[numClusters];
Expand Down Expand Up @@ -876,12 +855,26 @@ public void onNodeDisconnected(DiscoveryNode node, Transport.Connection connecti
}

private static void resolveWithEmptySearchResponse(Tuple<SearchRequest, ActionListener<SearchResponse>> tuple) {
var resp = emptySearchResponse();
try {
tuple.v2().onResponse(resp);
} finally {
resp.decRef();
}
ActionListener.respondAndRelease(
tuple.v2(),
new SearchResponse(
SearchHits.empty(new TotalHits(0, TotalHits.Relation.EQUAL_TO), Float.NaN),
InternalAggregations.EMPTY,
null,
false,
null,
null,
1,
null,
1,
1,
0,
100,
ShardSearchFailure.EMPTY_ARRAY,
SearchResponse.Clusters.EMPTY,
null
)
);
}

public void testCollectSearchShards() throws Exception {
Expand Down
Loading