Skip to content

Fix failure for validate API on a terms query #29483

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

Merged
merged 16 commits into from
May 1, 2018
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 @@ -75,7 +75,11 @@ public String getExplanation() {

@Override
public void readFrom(StreamInput in) throws IOException {
index = in.readString();
if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
index = in.readOptionalString();
} else {
index = in.readString();
}
if (in.getVersion().onOrAfter(Version.V_5_4_0)) {
shard = in.readInt();
} else {
Expand All @@ -88,7 +92,11 @@ public void readFrom(StreamInput in) throws IOException {

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(index);
if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) {
out.writeOptionalString(index);
} else {
out.writeString(index);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should back port this fix to 6.4. These version checks are fine to leave here for now but when you back port you will need to change them to Version.V_6_4_0 in the back ported commit on the 6.x branch an then after that add a new commit to the master branch to change these checks to Version.V_6_4_0 on master too. This process avoids getting CI failures before you've had a chance to back port the commit to 6.x.

if (out.getVersion().onOrAfter(Version.V_5_4_0)) {
out.writeInt(shard);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,11 @@
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.lease.Releasables;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.index.query.ParsedQuery;
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.index.query.Rewriteable;
import org.elasticsearch.indices.IndexClosedException;
import org.elasticsearch.search.SearchService;
import org.elasticsearch.search.internal.AliasFilter;
import org.elasticsearch.search.internal.SearchContext;
Expand All @@ -54,6 +57,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicReferenceArray;
import java.util.function.LongSupplier;

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

Expand All @@ -71,7 +75,39 @@ public TransportValidateQueryAction(Settings settings, ThreadPool threadPool, Cl
@Override
protected void doExecute(Task task, ValidateQueryRequest request, ActionListener<ValidateQueryResponse> listener) {
request.nowInMillis = System.currentTimeMillis();
super.doExecute(task, request, listener);
LongSupplier timeProvider = () -> request.nowInMillis;
ActionListener<org.elasticsearch.index.query.QueryBuilder> rewriteListener = ActionListener.wrap(rewrittenQuery -> {
request.query(rewrittenQuery);
super.doExecute(task, request, listener);
},
ex -> {
if (ex instanceof IndexNotFoundException ||
ex instanceof IndexClosedException) {
listener.onFailure(ex);
}
List<QueryExplanation> explanations = new ArrayList<>();
explanations.add(new QueryExplanation(null,
QueryExplanation.RANDOM_SHARD,
false,
null,
ex.getMessage()));
listener.onResponse(
new ValidateQueryResponse(
false,
explanations,
// totalShards is documented as "the total shards this request ran against",
// which is 0 since the failure is happening on the coordinating node.
0,
0 ,
0,
null));
});
if (request.query() == null) {
rewriteListener.onResponse(request.query());
} else {
Rewriteable.rewriteAndFetch(request.query(), searchService.getRewriteContext(timeProvider),
rewriteListener);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.elasticsearch.index.query;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.common.ParsingException;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -111,7 +112,7 @@ static <T extends Rewriteable<T>> void rewriteAndFetch(T original, QueryRewriteC
}
}
rewriteResponse.onResponse(builder);
} catch (IOException ex) {
} catch (IOException|IllegalArgumentException|ParsingException ex) {
rewriteResponse.onFailure(ex);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import org.elasticsearch.index.query.MoreLikeThisQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.TermsQueryBuilder;
import org.elasticsearch.indices.TermsLookup;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.ESIntegTestCase.ClusterScope;
import org.elasticsearch.test.ESIntegTestCase.Scope;
Expand Down Expand Up @@ -330,4 +332,21 @@ private static void assertExplanations(QueryBuilder queryBuilder,
assertThat(response.isValid(), equalTo(true));
}
}

public void testExplainTermsQueryWithLookup() throws Exception {
client().admin().indices().prepareCreate("twitter")
.addMapping("_doc", "user", "type=integer", "followers", "type=integer")
.setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 2).put("index.number_of_routing_shards", 2)).get();
client().prepareIndex("twitter", "_doc", "1")
.setSource("followers", new int[] {1, 2, 3}).get();
refresh();

TermsQueryBuilder termsLookupQuery = QueryBuilders.termsLookupQuery("user", new TermsLookup("twitter", "_doc", "1", "followers"));
ValidateQueryResponse response = client().admin().indices().prepareValidateQuery("twitter")
.setTypes("_doc")
.setQuery(termsLookupQuery)
.setExplain(true)
.execute().actionGet();
assertThat(response.isValid(), is(true));
}
}