Skip to content

Commit

Permalink
Validate query api: move query parsing on the coordinating node
Browse files Browse the repository at this point in the history
 Similarly to what we did with the search api, we can now also move query parsing on the coordinating node for the validate query api. Given that the explain api is a single shard operation (compared to search which is instead a broadcast operation), this doesn't change a lot in how the api works internally. The main benefit is that we can simplify the java api by requiring a structured query object to be provided rather than a bytes array that will get parsed on the data node. Previously if you specified a QueryBuilder it would be serialized in json format and would get reparsed on the data node, while now it doesn't go through parsing anymore (as expected), given that after the query-refactoring we are able to properly stream queries natively. Note that the WrapperQueryBuilder can be used from the java api to provide a query as a string, in that case the actual parsing of the inner query will happen on the data node.

Relates to elastic#10217
Closes elastic#14384
  • Loading branch information
javanna committed Nov 2, 2015
1 parent ebec4bd commit b56bbf6
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 264 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
import org.elasticsearch.action.support.broadcast.BroadcastShardRequest;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.shard.ShardId;

import java.io.IOException;
Expand All @@ -34,7 +34,7 @@
*/
public class ShardValidateQueryRequest extends BroadcastShardRequest {

private BytesReference source;
private QueryBuilder<?> query;
private String[] types = Strings.EMPTY_ARRAY;
private boolean explain;
private boolean rewrite;
Expand All @@ -49,16 +49,16 @@ public ShardValidateQueryRequest() {

ShardValidateQueryRequest(ShardId shardId, @Nullable String[] filteringAliases, ValidateQueryRequest request) {
super(shardId, request);
this.source = request.source();
this.query = request.query();
this.types = request.types();
this.explain = request.explain();
this.rewrite = request.rewrite();
this.filteringAliases = filteringAliases;
this.nowInMillis = request.nowInMillis;
}

public BytesReference source() {
return source;
public QueryBuilder<?> query() {
return query;
}

public String[] types() {
Expand All @@ -84,7 +84,7 @@ public long nowInMillis() {
@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
source = in.readBytesReference();
query = in.readQuery();

int typesSize = in.readVInt();
if (typesSize > 0) {
Expand All @@ -109,7 +109,7 @@ public void readFrom(StreamInput in) throws IOException {
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeBytesReference(source);
out.writeQuery(query);

out.writeVInt(types.length);
for (String type : types) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.routing.GroupShardsIterator;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.index.engine.Engine;
import org.elasticsearch.index.query.IndexQueryParserService;
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.index.shard.IndexShard;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.script.ScriptService;
Expand Down Expand Up @@ -178,9 +178,7 @@ protected ShardValidateQueryResponse shardOperation(ShardValidateQueryRequest re
);
SearchContext.setCurrent(searchContext);
try {
if (request.source() != null && request.source().length() > 0) {
searchContext.parsedQuery(queryParserService.parseTopLevelQuery(request.source()));
}
searchContext.parsedQuery(queryParserService.toQuery(request.query()));
searchContext.preProcess();

valid = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,27 @@

package org.elasticsearch.action.admin.indices.validate.query;

import org.elasticsearch.ElasticsearchGenerationException;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.ValidateActions;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.QuerySourceBuilder;
import org.elasticsearch.action.support.broadcast.BroadcastRequest;
import org.elasticsearch.client.Requests;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.index.query.MatchAllQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;

import java.io.IOException;
import java.util.Arrays;
import java.util.Map;

/**
* A request to validate a specific query.
* <p>
* The request requires the query source to be set either using {@link #source(QuerySourceBuilder)},
* or {@link #source(byte[])}.
* The request requires the query to be set using {@link #query(QueryBuilder)}
*/
public class ValidateQueryRequest extends BroadcastRequest<ValidateQueryRequest> {

private BytesReference source;
private QueryBuilder<?> query = new MatchAllQueryBuilder();

private boolean explain;
private boolean rewrite;
Expand All @@ -71,67 +64,21 @@ public ValidateQueryRequest(String... indices) {
@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = super.validate();
return validationException;
}

/**
* The source to execute.
*/
public BytesReference source() {
return source;
}

public ValidateQueryRequest source(QuerySourceBuilder sourceBuilder) {
this.source = sourceBuilder.buildAsBytes(Requests.CONTENT_TYPE);
return this;
}

/**
* The source to execute in the form of a map.
*/
public ValidateQueryRequest source(Map source) {
try {
XContentBuilder builder = XContentFactory.contentBuilder(Requests.CONTENT_TYPE);
builder.map(source);
return source(builder);
} catch (IOException e) {
throw new ElasticsearchGenerationException("Failed to generate [" + source + "]", e);
if (query == null) {
validationException = ValidateActions.addValidationError("query cannot be null", validationException);
}
}

public ValidateQueryRequest source(XContentBuilder builder) {
this.source = builder.bytes();
return this;
}

/**
* The query source to validate. It is preferable to use either {@link #source(byte[])}
* or {@link #source(QuerySourceBuilder)}.
*/
public ValidateQueryRequest source(String source) {
this.source = new BytesArray(source);
return this;
}

/**
* The source to validate.
*/
public ValidateQueryRequest source(byte[] source) {
return source(source, 0, source.length);
return validationException;
}

/**
* The source to validate.
* The query to validate.
*/
public ValidateQueryRequest source(byte[] source, int offset, int length) {
return source(new BytesArray(source, offset, length));
public QueryBuilder<?> query() {
return query;
}

/**
* The source to validate.
*/
public ValidateQueryRequest source(BytesReference source) {
this.source = source;
public ValidateQueryRequest query(QueryBuilder<?> query) {
this.query = query;
return this;
}

Expand Down Expand Up @@ -181,45 +128,33 @@ public boolean rewrite() {
@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);

source = in.readBytesReference();

query = in.readQuery();
int typesSize = in.readVInt();
if (typesSize > 0) {
types = new String[typesSize];
for (int i = 0; i < typesSize; i++) {
types[i] = in.readString();
}
}

explain = in.readBoolean();
rewrite = in.readBoolean();
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);

out.writeBytesReference(source);

out.writeQuery(query);
out.writeVInt(types.length);
for (String type : types) {
out.writeString(type);
}

out.writeBoolean(explain);
out.writeBoolean(rewrite);
}

@Override
public String toString() {
String sSource = "_na_";
try {
sSource = XContentHelper.convertToJson(source, false);
} catch (Exception e) {
// ignore
}
return "[" + Arrays.toString(indices) + "]" + Arrays.toString(types) + ", source[" + sSource + "], explain:" + explain +
return "[" + Arrays.toString(indices) + "]" + Arrays.toString(types) + ", query[" + query + "], explain:" + explain +
", rewrite:" + rewrite;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,15 @@

package org.elasticsearch.action.admin.indices.validate.query;

import org.elasticsearch.action.support.QuerySourceBuilder;
import org.elasticsearch.action.support.broadcast.BroadcastOperationRequestBuilder;
import org.elasticsearch.client.ElasticsearchClient;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.index.query.QueryBuilder;

/**
*
*/
public class ValidateQueryRequestBuilder extends BroadcastOperationRequestBuilder<ValidateQueryRequest, ValidateQueryResponse, ValidateQueryRequestBuilder> {

private QuerySourceBuilder sourceBuilder;

public ValidateQueryRequestBuilder(ElasticsearchClient client, ValidateQueryAction action) {
super(client, action, new ValidateQueryRequest());
}
Expand All @@ -45,32 +41,12 @@ public ValidateQueryRequestBuilder setTypes(String... types) {
}

/**
* The query source to validate.
* The query to validate.
*
* @see org.elasticsearch.index.query.QueryBuilders
*/
public ValidateQueryRequestBuilder setQuery(QueryBuilder queryBuilder) {
sourceBuilder().setQuery(queryBuilder);
return this;
}

/**
* The source to validate.
*
* @see org.elasticsearch.index.query.QueryBuilders
*/
public ValidateQueryRequestBuilder setSource(BytesReference source) {
request().source(source);
return this;
}

/**
* The source to validate.
*
* @see org.elasticsearch.index.query.QueryBuilders
*/
public ValidateQueryRequestBuilder setSource(byte[] source) {
request.source(source);
request.query(queryBuilder);
return this;
}

Expand All @@ -91,19 +67,4 @@ public ValidateQueryRequestBuilder setRewrite(boolean rewrite) {
request.rewrite(rewrite);
return this;
}

@Override
protected ValidateQueryRequest beforeExecute(ValidateQueryRequest request) {
if (sourceBuilder != null) {
request.source(sourceBuilder);
}
return request;
}

private QuerySourceBuilder sourceBuilder() {
if (sourceBuilder == null) {
sourceBuilder = new QuerySourceBuilder();
}
return sourceBuilder;
}
}

This file was deleted.

Loading

0 comments on commit b56bbf6

Please sign in to comment.