Fixed validate query parsing issues
Made sure that a match_all query is used when no query is specified and ensure no NPE is thrown either. Also used the same code path as the search api to ensure that alias filters are taken into account, same for type filters. Closes #6111 Closes #6112 Closes #6116
This commit is contained in:
parent
513f25ae97
commit
3d63bac51d
|
@ -25,3 +25,12 @@
|
|||
|
||||
- is_false: valid
|
||||
|
||||
- do:
|
||||
indices.validate_query:
|
||||
explain: true
|
||||
|
||||
- is_true: valid
|
||||
- match: {_shards.failed: 0}
|
||||
- match: {explanations.0.index: 'testing'}
|
||||
- match: {explanations.0.explanation: 'ConstantScore(*:*)'}
|
||||
|
||||
|
|
|
@ -37,7 +37,6 @@ import org.elasticsearch.common.inject.Inject;
|
|||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.util.BigArrays;
|
||||
import org.elasticsearch.index.query.IndexQueryParserService;
|
||||
import org.elasticsearch.index.query.ParsedQuery;
|
||||
import org.elasticsearch.index.query.QueryParsingException;
|
||||
import org.elasticsearch.index.service.IndexService;
|
||||
import org.elasticsearch.index.shard.service.IndexShard;
|
||||
|
@ -182,30 +181,35 @@ public class TransportValidateQueryAction extends TransportBroadcastOperationAct
|
|||
boolean valid;
|
||||
String explanation = null;
|
||||
String error = null;
|
||||
if (request.source().length() == 0) {
|
||||
valid = true;
|
||||
} else {
|
||||
SearchContext.setCurrent(new DefaultSearchContext(0,
|
||||
new ShardSearchRequest().types(request.types()).nowInMillis(request.nowInMillis()),
|
||||
null, indexShard.acquireSearcher("validate_query"), indexService, indexShard,
|
||||
scriptService, cacheRecycler, pageCacheRecycler, bigArrays));
|
||||
try {
|
||||
ParsedQuery parsedQuery = queryParserService.parseQuery(request.source());
|
||||
valid = true;
|
||||
if (request.explain()) {
|
||||
explanation = parsedQuery.query().toString();
|
||||
}
|
||||
} catch (QueryParsingException e) {
|
||||
valid = false;
|
||||
error = e.getDetailedMessage();
|
||||
} catch (AssertionError e) {
|
||||
valid = false;
|
||||
error = e.getMessage();
|
||||
} finally {
|
||||
SearchContext.current().close();
|
||||
SearchContext.removeCurrent();
|
||||
|
||||
DefaultSearchContext searchContext = new DefaultSearchContext(0,
|
||||
new ShardSearchRequest().types(request.types()).nowInMillis(request.nowInMillis())
|
||||
.filteringAliases(request.filteringAliases()),
|
||||
null, indexShard.acquireSearcher("validate_query"), indexService, indexShard,
|
||||
scriptService, cacheRecycler, pageCacheRecycler, bigArrays
|
||||
);
|
||||
SearchContext.setCurrent(searchContext);
|
||||
try {
|
||||
if (request.source() != null && request.source().length() > 0) {
|
||||
searchContext.parsedQuery(queryParserService.parseQuery(request.source()));
|
||||
}
|
||||
searchContext.preProcess();
|
||||
|
||||
valid = true;
|
||||
if (request.explain()) {
|
||||
explanation = searchContext.query().toString();
|
||||
}
|
||||
} catch (QueryParsingException e) {
|
||||
valid = false;
|
||||
error = e.getDetailedMessage();
|
||||
} catch (AssertionError e) {
|
||||
valid = false;
|
||||
error = e.getMessage();
|
||||
} finally {
|
||||
SearchContext.current().close();
|
||||
SearchContext.removeCurrent();
|
||||
}
|
||||
|
||||
return new ShardValidateQueryResponse(request.index(), request.shardId(), valid, explanation, error);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -19,6 +19,7 @@
|
|||
package org.elasticsearch.validate;
|
||||
|
||||
import com.google.common.base.Charsets;
|
||||
import org.elasticsearch.action.admin.indices.alias.Alias;
|
||||
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryResponse;
|
||||
import org.elasticsearch.client.Client;
|
||||
import org.elasticsearch.common.geo.GeoDistance;
|
||||
|
@ -28,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentFactory;
|
|||
import org.elasticsearch.index.query.FilterBuilders;
|
||||
import org.elasticsearch.index.query.QueryBuilder;
|
||||
import org.elasticsearch.index.query.QueryBuilders;
|
||||
import org.elasticsearch.indices.IndexMissingException;
|
||||
import org.elasticsearch.test.ElasticsearchIntegrationTest;
|
||||
import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope;
|
||||
import org.hamcrest.Matcher;
|
||||
|
@ -107,12 +109,12 @@ public class SimpleValidateQueryTests extends ElasticsearchIntegrationTest {
|
|||
assertThat(response.getQueryExplanation().get(0).getError(), containsString("Failed to parse"));
|
||||
assertThat(response.getQueryExplanation().get(0).getExplanation(), nullValue());
|
||||
|
||||
assertExplanation(QueryBuilders.queryString("_id:1"), equalTo("ConstantScore(_uid:type1#1)"));
|
||||
assertExplanation(QueryBuilders.queryString("_id:1"), equalTo("filtered(ConstantScore(_uid:type1#1))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.idsQuery("type1").addIds("1").addIds("2"),
|
||||
equalTo("ConstantScore(_uid:type1#1 _uid:type1#2)"));
|
||||
equalTo("filtered(ConstantScore(_uid:type1#1 _uid:type1#2))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.queryString("foo"), equalTo("_all:foo"));
|
||||
assertExplanation(QueryBuilders.queryString("foo"), equalTo("filtered(_all:foo)->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.filteredQuery(
|
||||
QueryBuilders.termQuery("foo", "1"),
|
||||
|
@ -120,14 +122,14 @@ public class SimpleValidateQueryTests extends ElasticsearchIntegrationTest {
|
|||
FilterBuilders.termFilter("bar", "2"),
|
||||
FilterBuilders.termFilter("baz", "3")
|
||||
)
|
||||
), equalTo("filtered(foo:1)->cache(bar:[2 TO 2]) cache(baz:3)"));
|
||||
), equalTo("filtered(filtered(foo:1)->cache(bar:[2 TO 2]) cache(baz:3))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.filteredQuery(
|
||||
QueryBuilders.termQuery("foo", "1"),
|
||||
FilterBuilders.orFilter(
|
||||
FilterBuilders.termFilter("bar", "2")
|
||||
)
|
||||
), equalTo("filtered(foo:1)->cache(bar:[2 TO 2])"));
|
||||
), equalTo("filtered(filtered(foo:1)->cache(bar:[2 TO 2]))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.filteredQuery(
|
||||
QueryBuilders.matchAllQuery(),
|
||||
|
@ -136,28 +138,28 @@ public class SimpleValidateQueryTests extends ElasticsearchIntegrationTest {
|
|||
.addPoint(30, -80)
|
||||
.addPoint(20, -90)
|
||||
.addPoint(40, -70) // closing polygon
|
||||
), equalTo("ConstantScore(GeoPolygonFilter(pin.location, [[40.0, -70.0], [30.0, -80.0], [20.0, -90.0], [40.0, -70.0]]))"));
|
||||
), equalTo("filtered(ConstantScore(GeoPolygonFilter(pin.location, [[40.0, -70.0], [30.0, -80.0], [20.0, -90.0], [40.0, -70.0]])))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.constantScoreQuery(FilterBuilders.geoBoundingBoxFilter("pin.location")
|
||||
.topLeft(40, -80)
|
||||
.bottomRight(20, -70)
|
||||
), equalTo("ConstantScore(GeoBoundingBoxFilter(pin.location, [40.0, -80.0], [20.0, -70.0]))"));
|
||||
), equalTo("filtered(ConstantScore(GeoBoundingBoxFilter(pin.location, [40.0, -80.0], [20.0, -70.0])))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.constantScoreQuery(FilterBuilders.geoDistanceFilter("pin.location")
|
||||
.lat(10).lon(20).distance(15, DistanceUnit.DEFAULT).geoDistance(GeoDistance.PLANE)
|
||||
), equalTo("ConstantScore(GeoDistanceFilter(pin.location, PLANE, 15.0, 10.0, 20.0))"));
|
||||
), equalTo("filtered(ConstantScore(GeoDistanceFilter(pin.location, PLANE, 15.0, 10.0, 20.0)))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.constantScoreQuery(FilterBuilders.geoDistanceFilter("pin.location")
|
||||
.lat(10).lon(20).distance(15, DistanceUnit.DEFAULT).geoDistance(GeoDistance.PLANE)
|
||||
), equalTo("ConstantScore(GeoDistanceFilter(pin.location, PLANE, 15.0, 10.0, 20.0))"));
|
||||
), equalTo("filtered(ConstantScore(GeoDistanceFilter(pin.location, PLANE, 15.0, 10.0, 20.0)))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.constantScoreQuery(FilterBuilders.geoDistanceRangeFilter("pin.location")
|
||||
.lat(10).lon(20).from("15m").to("25m").geoDistance(GeoDistance.PLANE)
|
||||
), equalTo("ConstantScore(GeoDistanceRangeFilter(pin.location, PLANE, [15.0 - 25.0], 10.0, 20.0))"));
|
||||
), equalTo("filtered(ConstantScore(GeoDistanceRangeFilter(pin.location, PLANE, [15.0 - 25.0], 10.0, 20.0)))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.constantScoreQuery(FilterBuilders.geoDistanceRangeFilter("pin.location")
|
||||
.lat(10).lon(20).from("15miles").to("25miles").geoDistance(GeoDistance.PLANE)
|
||||
), equalTo("ConstantScore(GeoDistanceRangeFilter(pin.location, PLANE, [" + DistanceUnit.DEFAULT.convert(15.0, DistanceUnit.MILES) + " - " + DistanceUnit.DEFAULT.convert(25.0, DistanceUnit.MILES) + "], 10.0, 20.0))"));
|
||||
), equalTo("filtered(ConstantScore(GeoDistanceRangeFilter(pin.location, PLANE, [" + DistanceUnit.DEFAULT.convert(15.0, DistanceUnit.MILES) + " - " + DistanceUnit.DEFAULT.convert(25.0, DistanceUnit.MILES) + "], 10.0, 20.0)))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.filteredQuery(
|
||||
QueryBuilders.termQuery("foo", "1"),
|
||||
|
@ -165,13 +167,13 @@ public class SimpleValidateQueryTests extends ElasticsearchIntegrationTest {
|
|||
FilterBuilders.termFilter("bar", "2"),
|
||||
FilterBuilders.termFilter("baz", "3")
|
||||
)
|
||||
), equalTo("filtered(foo:1)->+cache(bar:[2 TO 2]) +cache(baz:3)"));
|
||||
), equalTo("filtered(filtered(foo:1)->+cache(bar:[2 TO 2]) +cache(baz:3))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.constantScoreQuery(FilterBuilders.termsFilter("foo", "1", "2", "3")),
|
||||
equalTo("ConstantScore(cache(foo:1 foo:2 foo:3))"));
|
||||
equalTo("filtered(ConstantScore(cache(foo:1 foo:2 foo:3)))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.constantScoreQuery(FilterBuilders.notFilter(FilterBuilders.termFilter("foo", "bar"))),
|
||||
equalTo("ConstantScore(NotFilter(cache(foo:bar)))"));
|
||||
equalTo("filtered(ConstantScore(NotFilter(cache(foo:bar))))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.filteredQuery(
|
||||
QueryBuilders.termQuery("foo", "1"),
|
||||
|
@ -179,12 +181,12 @@ public class SimpleValidateQueryTests extends ElasticsearchIntegrationTest {
|
|||
"child-type",
|
||||
QueryBuilders.matchQuery("foo", "1")
|
||||
)
|
||||
), equalTo("filtered(foo:1)->CustomQueryWrappingFilter(child_filter[child-type/type1](filtered(foo:1)->cache(_type:child-type)))"));
|
||||
), equalTo("filtered(filtered(foo:1)->CustomQueryWrappingFilter(child_filter[child-type/type1](filtered(foo:1)->cache(_type:child-type))))->cache(_type:type1)"));
|
||||
|
||||
assertExplanation(QueryBuilders.filteredQuery(
|
||||
QueryBuilders.termQuery("foo", "1"),
|
||||
FilterBuilders.scriptFilter("true")
|
||||
), equalTo("filtered(foo:1)->ScriptFilter(true)"));
|
||||
), equalTo("filtered(filtered(foo:1)->ScriptFilter(true))->cache(_type:type1)"));
|
||||
|
||||
}
|
||||
|
||||
|
@ -253,6 +255,38 @@ public class SimpleValidateQueryTests extends ElasticsearchIntegrationTest {
|
|||
assertThat(response.isValid(), equalTo(true));
|
||||
}
|
||||
|
||||
@Test(expected = IndexMissingException.class)
|
||||
public void validateEmptyCluster() {
|
||||
client().admin().indices().prepareValidateQuery().get();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void explainNoQuery() {
|
||||
createIndex("test");
|
||||
ensureGreen();
|
||||
|
||||
ValidateQueryResponse validateQueryResponse = client().admin().indices().prepareValidateQuery().setExplain(true).get();
|
||||
assertThat(validateQueryResponse.isValid(), equalTo(true));
|
||||
assertThat(validateQueryResponse.getQueryExplanation().size(), equalTo(1));
|
||||
assertThat(validateQueryResponse.getQueryExplanation().get(0).getIndex(), equalTo("test"));
|
||||
assertThat(validateQueryResponse.getQueryExplanation().get(0).getExplanation(), equalTo("ConstantScore(*:*)"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void explainFilteredAlias() {
|
||||
assertAcked(prepareCreate("test")
|
||||
.addMapping("test", "field", "type=string")
|
||||
.addAlias(new Alias("alias").filter(FilterBuilders.termFilter("field", "value1"))));
|
||||
ensureGreen();
|
||||
|
||||
ValidateQueryResponse validateQueryResponse = client().admin().indices().prepareValidateQuery("alias")
|
||||
.setQuery(QueryBuilders.matchAllQuery()).setExplain(true).get();
|
||||
assertThat(validateQueryResponse.isValid(), equalTo(true));
|
||||
assertThat(validateQueryResponse.getQueryExplanation().size(), equalTo(1));
|
||||
assertThat(validateQueryResponse.getQueryExplanation().get(0).getIndex(), equalTo("test"));
|
||||
assertThat(validateQueryResponse.getQueryExplanation().get(0).getExplanation(), containsString("field:value1"));
|
||||
}
|
||||
|
||||
private void assertExplanation(QueryBuilder queryBuilder, Matcher<String> matcher) {
|
||||
ValidateQueryResponse response = client().admin().indices().prepareValidateQuery("test")
|
||||
.setTypes("type1")
|
||||
|
|
Loading…
Reference in New Issue