Skip to content

Commit 00b21f8

Browse files
author
Paul Sanwald
authored
Fix failure for validate API on a terms query (#29483)
* WIP commit to try calling rewrite on coordinating node during TransportSearchAction * Use re-written query instead of using the original query * fix incorrect/unused imports and wildcarding * add error handling for cases where an exception is thrown * correct exception handling such that integration tests pass successfully * fix additional case covered by IndicesOptionsIntegrationIT. * add integration test case that verifies queries are now valid * add optional value for index * address review comments: catch superclass of XContentParseException fixes #29483
1 parent c5dc607 commit 00b21f8

File tree

4 files changed

+68
-4
lines changed

4 files changed

+68
-4
lines changed

server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/QueryExplanation.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,11 @@ public String getExplanation() {
7575

7676
@Override
7777
public void readFrom(StreamInput in) throws IOException {
78-
index = in.readString();
78+
if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
79+
index = in.readOptionalString();
80+
} else {
81+
index = in.readString();
82+
}
7983
if (in.getVersion().onOrAfter(Version.V_5_4_0)) {
8084
shard = in.readInt();
8185
} else {
@@ -88,7 +92,11 @@ public void readFrom(StreamInput in) throws IOException {
8892

8993
@Override
9094
public void writeTo(StreamOutput out) throws IOException {
91-
out.writeString(index);
95+
if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
96+
out.writeOptionalString(index);
97+
} else {
98+
out.writeString(index);
99+
}
92100
if (out.getVersion().onOrAfter(Version.V_5_4_0)) {
93101
out.writeInt(shard);
94102
}

server/src/main/java/org/elasticsearch/action/admin/indices/validate/query/TransportValidateQueryAction.java

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,11 @@
3838
import org.elasticsearch.common.inject.Inject;
3939
import org.elasticsearch.common.lease.Releasables;
4040
import org.elasticsearch.common.settings.Settings;
41+
import org.elasticsearch.index.IndexNotFoundException;
4142
import org.elasticsearch.index.query.ParsedQuery;
4243
import org.elasticsearch.index.query.QueryShardException;
44+
import org.elasticsearch.index.query.Rewriteable;
45+
import org.elasticsearch.indices.IndexClosedException;
4346
import org.elasticsearch.search.SearchService;
4447
import org.elasticsearch.search.internal.AliasFilter;
4548
import org.elasticsearch.search.internal.SearchContext;
@@ -54,6 +57,7 @@
5457
import java.util.Map;
5558
import java.util.Set;
5659
import java.util.concurrent.atomic.AtomicReferenceArray;
60+
import java.util.function.LongSupplier;
5761

5862
public class TransportValidateQueryAction extends TransportBroadcastAction<ValidateQueryRequest, ValidateQueryResponse, ShardValidateQueryRequest, ShardValidateQueryResponse> {
5963

@@ -71,7 +75,39 @@ public TransportValidateQueryAction(Settings settings, ThreadPool threadPool, Cl
7175
@Override
7276
protected void doExecute(Task task, ValidateQueryRequest request, ActionListener<ValidateQueryResponse> listener) {
7377
request.nowInMillis = System.currentTimeMillis();
74-
super.doExecute(task, request, listener);
78+
LongSupplier timeProvider = () -> request.nowInMillis;
79+
ActionListener<org.elasticsearch.index.query.QueryBuilder> rewriteListener = ActionListener.wrap(rewrittenQuery -> {
80+
request.query(rewrittenQuery);
81+
super.doExecute(task, request, listener);
82+
},
83+
ex -> {
84+
if (ex instanceof IndexNotFoundException ||
85+
ex instanceof IndexClosedException) {
86+
listener.onFailure(ex);
87+
}
88+
List<QueryExplanation> explanations = new ArrayList<>();
89+
explanations.add(new QueryExplanation(null,
90+
QueryExplanation.RANDOM_SHARD,
91+
false,
92+
null,
93+
ex.getMessage()));
94+
listener.onResponse(
95+
new ValidateQueryResponse(
96+
false,
97+
explanations,
98+
// totalShards is documented as "the total shards this request ran against",
99+
// which is 0 since the failure is happening on the coordinating node.
100+
0,
101+
0 ,
102+
0,
103+
null));
104+
});
105+
if (request.query() == null) {
106+
rewriteListener.onResponse(request.query());
107+
} else {
108+
Rewriteable.rewriteAndFetch(request.query(), searchService.getRewriteContext(timeProvider),
109+
rewriteListener);
110+
}
75111
}
76112

77113
@Override

server/src/main/java/org/elasticsearch/index/query/Rewriteable.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.elasticsearch.index.query;
2020

2121
import org.elasticsearch.action.ActionListener;
22+
import org.elasticsearch.common.ParsingException;
2223

2324
import java.io.IOException;
2425
import java.util.ArrayList;
@@ -111,7 +112,7 @@ static <T extends Rewriteable<T>> void rewriteAndFetch(T original, QueryRewriteC
111112
}
112113
}
113114
rewriteResponse.onResponse(builder);
114-
} catch (IOException ex) {
115+
} catch (IOException|IllegalArgumentException|ParsingException ex) {
115116
rewriteResponse.onFailure(ex);
116117
}
117118
}

server/src/test/java/org/elasticsearch/validate/SimpleValidateQueryIT.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import org.elasticsearch.index.query.MoreLikeThisQueryBuilder;
3030
import org.elasticsearch.index.query.QueryBuilder;
3131
import org.elasticsearch.index.query.QueryBuilders;
32+
import org.elasticsearch.index.query.TermsQueryBuilder;
33+
import org.elasticsearch.indices.TermsLookup;
3234
import org.elasticsearch.test.ESIntegTestCase;
3335
import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
3436
import org.elasticsearch.test.ESIntegTestCase.Scope;
@@ -330,4 +332,21 @@ private static void assertExplanations(QueryBuilder queryBuilder,
330332
assertThat(response.isValid(), equalTo(true));
331333
}
332334
}
335+
336+
public void testExplainTermsQueryWithLookup() throws Exception {
337+
client().admin().indices().prepareCreate("twitter")
338+
.addMapping("_doc", "user", "type=integer", "followers", "type=integer")
339+
.setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 2).put("index.number_of_routing_shards", 2)).get();
340+
client().prepareIndex("twitter", "_doc", "1")
341+
.setSource("followers", new int[] {1, 2, 3}).get();
342+
refresh();
343+
344+
TermsQueryBuilder termsLookupQuery = QueryBuilders.termsLookupQuery("user", new TermsLookup("twitter", "_doc", "1", "followers"));
345+
ValidateQueryResponse response = client().admin().indices().prepareValidateQuery("twitter")
346+
.setTypes("_doc")
347+
.setQuery(termsLookupQuery)
348+
.setExplain(true)
349+
.execute().actionGet();
350+
assertThat(response.isValid(), is(true));
351+
}
333352
}

0 commit comments

Comments
 (0)