From 9dd0018404eee751f0602a60fd11d10ce684c0d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 29 Oct 2019 13:26:41 +0100 Subject: [PATCH] Support `search_type` in Rank Evaluation API (#48542) Adding support for the `search_type` request parameter to the Ranking Evaluation API since this parameter can impact the ranking and the metric score and should be choosen in the same way when evaluating the search as later in the real search. Closes #48503 --- .../client/RequestConverters.java | 1 + .../client/RequestConvertersTests.java | 6 +- .../index/rankeval/RankEvalRequest.java | 51 ++++++------ .../index/rankeval/RestRankEvalAction.java | 4 + .../rankeval/TransportRankEvalAction.java | 1 + .../index/rankeval/RankEvalRequestTests.java | 11 +++ .../TransportRankEvalActionTests.java | 78 +++++++++++++++++++ .../rest-api-spec/test/rank_eval/10_basic.yml | 1 + .../rest-api-spec/api/rank_eval.json | 8 ++ 9 files changed, 137 insertions(+), 24 deletions(-) create mode 100644 modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/TransportRankEvalActionTests.java diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java index bf2814edbbc20..142d411d2aef0 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java @@ -538,6 +538,7 @@ static Request rankEval(RankEvalRequest rankEvalRequest) throws IOException { Params params = new Params(); params.withIndicesOptions(rankEvalRequest.indicesOptions()); + params.putParam("search_type", rankEvalRequest.searchType().name().toLowerCase(Locale.ROOT)); request.addParameters(params.asMap()); request.setEntity(createEntity(rankEvalRequest.getRankEvalSpec(), REQUEST_BODY_CONTENT_TYPE)); return request; diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java index 41f3d86123d92..f02e62eefcf99 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java @@ -1642,6 +1642,10 @@ public void testRankEval() throws Exception { RankEvalRequest rankEvalRequest = new RankEvalRequest(spec, indices); Map expectedParams = new HashMap<>(); setRandomIndicesOptions(rankEvalRequest::indicesOptions, rankEvalRequest::indicesOptions, expectedParams); + if (randomBoolean()) { + rankEvalRequest.searchType(randomFrom(SearchType.CURRENTLY_SUPPORTED)); + } + expectedParams.put("search_type", rankEvalRequest.searchType().name().toLowerCase(Locale.ROOT)); Request request = RequestConverters.rankEval(rankEvalRequest); StringJoiner endpoint = new StringJoiner("/", "/", ""); @@ -1651,7 +1655,7 @@ public void testRankEval() throws Exception { } endpoint.add(RestRankEvalAction.ENDPOINT); assertEquals(endpoint.toString(), request.getEndpoint()); - assertEquals(4, request.getParameters().size()); + assertEquals(5, request.getParameters().size()); assertEquals(expectedParams, request.getParameters()); assertToXContentBody(spec, request.getEntity()); } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalRequest.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalRequest.java index 71a2cd433104f..269d8d338296a 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalRequest.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RankEvalRequest.java @@ -24,6 +24,7 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.search.SearchRequest; +import org.elasticsearch.action.search.SearchType; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -43,6 +44,8 @@ public class RankEvalRequest extends ActionRequest implements IndicesRequest.Rep private IndicesOptions indicesOptions = SearchRequest.DEFAULT_INDICES_OPTIONS; private String[] indices = Strings.EMPTY_ARRAY; + private SearchType searchType = SearchType.DEFAULT; + public RankEvalRequest(RankEvalSpec rankingEvaluationSpec, String[] indices) { this.rankingEvaluationSpec = Objects.requireNonNull(rankingEvaluationSpec, "ranking evaluation specification must not be null"); indices(indices); @@ -51,17 +54,10 @@ public RankEvalRequest(RankEvalSpec rankingEvaluationSpec, String[] indices) { RankEvalRequest(StreamInput in) throws IOException { super(in); rankingEvaluationSpec = new RankEvalSpec(in); - if (in.getVersion().onOrAfter(Version.V_6_3_0)) { - indices = in.readStringArray(); - indicesOptions = IndicesOptions.readIndicesOptions(in); - } else { - // readStringArray uses readVInt for size, we used readInt in 6.2 - int indicesSize = in.readInt(); - String[] indices = new String[indicesSize]; - for (int i = 0; i < indicesSize; i++) { - indices[i] = in.readString(); - } - // no indices options yet + indices = in.readStringArray(); + indicesOptions = IndicesOptions.readIndicesOptions(in); + if (in.getVersion().onOrAfter(Version.V_7_6_0)) { + searchType = SearchType.fromId(in.readByte()); } } @@ -122,20 +118,28 @@ public void indicesOptions(IndicesOptions indicesOptions) { this.indicesOptions = Objects.requireNonNull(indicesOptions, "indicesOptions must not be null"); } + /** + * The search type to execute, defaults to {@link SearchType#DEFAULT}. + */ + public void searchType(SearchType searchType) { + this.searchType = Objects.requireNonNull(searchType, "searchType must not be null"); + } + + /** + * The type of search to execute. + */ + public SearchType searchType() { + return searchType; + } + @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); rankingEvaluationSpec.writeTo(out); - if (out.getVersion().onOrAfter(Version.V_6_3_0)) { - out.writeStringArray(indices); - indicesOptions.writeIndicesOptions(out); - } else { - // writeStringArray uses writeVInt for size, we used writeInt in 6.2 - out.writeInt(indices.length); - for (String index : indices) { - out.writeString(index); - } - // no indices options yet + out.writeStringArray(indices); + indicesOptions.writeIndicesOptions(out); + if (out.getVersion().onOrAfter(Version.V_7_6_0)) { + out.writeByte(searchType.id()); } } @@ -150,11 +154,12 @@ public boolean equals(Object o) { RankEvalRequest that = (RankEvalRequest) o; return Objects.equals(indicesOptions, that.indicesOptions) && Arrays.equals(indices, that.indices) && - Objects.equals(rankingEvaluationSpec, that.rankingEvaluationSpec); + Objects.equals(rankingEvaluationSpec, that.rankingEvaluationSpec) && + Objects.equals(searchType, that.searchType); } @Override public int hashCode() { - return Objects.hash(indicesOptions, Arrays.hashCode(indices), rankingEvaluationSpec); + return Objects.hash(indicesOptions, Arrays.hashCode(indices), rankingEvaluationSpec, searchType); } } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RestRankEvalAction.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RestRankEvalAction.java index b4876eafe9372..85e9e656808e8 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RestRankEvalAction.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/RestRankEvalAction.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.rankeval; +import org.elasticsearch.action.search.SearchType; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.Strings; @@ -109,6 +110,9 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli private static void parseRankEvalRequest(RankEvalRequest rankEvalRequest, RestRequest request, XContentParser parser) { rankEvalRequest.indices(Strings.splitStringByCommaToArray(request.param("index"))); rankEvalRequest.indicesOptions(IndicesOptions.fromRequest(request, rankEvalRequest.indicesOptions())); + if (request.hasParam("search_type")) { + rankEvalRequest.searchType(SearchType.fromString(request.param("search_type"))); + } RankEvalSpec spec = RankEvalSpec.parse(parser); rankEvalRequest.setRankEvalSpec(spec); } diff --git a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java index b3c8f434f2612..b08e457e8f44b 100644 --- a/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java +++ b/modules/rank-eval/src/main/java/org/elasticsearch/index/rankeval/TransportRankEvalAction.java @@ -129,6 +129,7 @@ LoggingDeprecationHandler.INSTANCE, new BytesArray(resolvedRequest), XContentTyp } SearchRequest searchRequest = new SearchRequest(request.indices(), evaluationRequest); searchRequest.indicesOptions(request.indicesOptions()); + searchRequest.searchType(request.searchType()); msearchRequest.add(searchRequest); } assert ratedRequestsInSearch.size() == msearchRequest.requests().size(); diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java index 1a16c311fcf3a..7409a84e05629 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RankEvalRequestTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.rankeval; +import org.elasticsearch.action.search.SearchType; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.Writeable.Reader; @@ -62,6 +63,7 @@ protected RankEvalRequest createTestInstance() { randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean()); rankEvalRequest.indicesOptions(indicesOptions); + rankEvalRequest.searchType(randomFrom(SearchType.DFS_QUERY_THEN_FETCH, SearchType.QUERY_THEN_FETCH)); return rankEvalRequest; } @@ -77,8 +79,17 @@ protected RankEvalRequest mutateInstance(RankEvalRequest instance) throws IOExce mutators.add(() -> mutation.indices(ArrayUtils.concat(instance.indices(), new String[] { randomAlphaOfLength(10) }))); mutators.add(() -> mutation.indicesOptions(randomValueOtherThan(instance.indicesOptions(), () -> IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean())))); + mutators.add(() -> { + if (instance.searchType() == SearchType.DFS_QUERY_THEN_FETCH) { + mutation.searchType(SearchType.QUERY_THEN_FETCH); + } else { + mutation.searchType(SearchType.DFS_QUERY_THEN_FETCH); + } + }); + mutators.add(() -> mutation.setRankEvalSpec(RankEvalSpecTests.mutateTestItem(instance.getRankEvalSpec()))); mutators.add(() -> mutation.setRankEvalSpec(RankEvalSpecTests.mutateTestItem(instance.getRankEvalSpec()))); randomFrom(mutators).run(); return mutation; } + } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/TransportRankEvalActionTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/TransportRankEvalActionTests.java new file mode 100644 index 0000000000000..2e1199dcafa09 --- /dev/null +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/TransportRankEvalActionTests.java @@ -0,0 +1,78 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.rankeval; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.search.MultiSearchRequest; +import org.elasticsearch.action.search.MultiSearchResponse; +import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.action.support.IndicesOptions; +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.env.Environment; +import org.elasticsearch.script.ScriptService; +import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.transport.TransportService; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import static org.mockito.Mockito.mock; + +public class TransportRankEvalActionTests extends ESTestCase { + + private Settings settings = Settings.builder().put("path.home", createTempDir().toString()).put("node.name", "test-" + getTestName()) + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build(); + + /** + * Test that request parameters like indicesOptions or searchType from ranking evaluation request are transfered to msearch request + */ + public void testTransferRequestParameters() throws Exception { + String indexName = "test_index"; + List specifications = new ArrayList<>(); + specifications + .add(new RatedRequest("amsterdam_query", Arrays.asList(new RatedDocument(indexName, "1", 3)), new SearchSourceBuilder())); + RankEvalRequest rankEvalRequest = new RankEvalRequest(new RankEvalSpec(specifications, new DiscountedCumulativeGain()), + new String[] { indexName }); + SearchType expectedSearchType = randomFrom(SearchType.CURRENTLY_SUPPORTED); + rankEvalRequest.searchType(expectedSearchType); + IndicesOptions expectedIndicesOptions = IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(), + randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean(), randomBoolean()); + rankEvalRequest.indicesOptions(expectedIndicesOptions); + + NodeClient client = new NodeClient(settings, null) { + @Override + public void multiSearch(MultiSearchRequest request, ActionListener listener) { + assertEquals(1, request.requests().size()); + assertEquals(expectedSearchType, request.requests().get(0).searchType()); + assertArrayEquals(new String[]{indexName}, request.requests().get(0).indices()); + assertEquals(expectedIndicesOptions, request.requests().get(0).indicesOptions()); + } + }; + + TransportRankEvalAction action = new TransportRankEvalAction(mock(ActionFilters.class), client, mock(TransportService.class), + mock(ScriptService.class), NamedXContentRegistry.EMPTY); + action.doExecute(null, rankEvalRequest, null); + } +} diff --git a/modules/rank-eval/src/test/resources/rest-api-spec/test/rank_eval/10_basic.yml b/modules/rank-eval/src/test/resources/rest-api-spec/test/rank_eval/10_basic.yml index b9001cb782a80..382b0789ba0ec 100644 --- a/modules/rank-eval/src/test/resources/rest-api-spec/test/rank_eval/10_basic.yml +++ b/modules/rank-eval/src/test/resources/rest-api-spec/test/rank_eval/10_basic.yml @@ -48,6 +48,7 @@ setup: - do: rank_eval: index: foo, + search_type: query_then_fetch body: { "requests" : [ { diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/rank_eval.json b/rest-api-spec/src/main/resources/rest-api-spec/api/rank_eval.json index f22f4803bb489..c87b850ab148f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/rank_eval.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/rank_eval.json @@ -48,6 +48,14 @@ ], "default":"open", "description":"Whether to expand wildcard expression to concrete indices that are open, closed or both." + }, + "search_type":{ + "type":"enum", + "options":[ + "query_then_fetch", + "dfs_query_then_fetch" + ], + "description":"Search operation type" } }, "body":{