An `exists` query on an object should query a single term.

Currently if you run an `exists` query on an object, it will resolve all sub
fields and create a disjunction for all those fields. However the `_field_names`
mapper indexes paths for objects so we could query object paths directly.

I also changed the query parser to reject `exists` queries if the `_field_names`
field is disabled since it would be a big performance trap.
This commit is contained in:
Adrien Grand 2016-03-18 12:20:06 +01:00
parent b42f66c8ac
commit c52b1f3a7c
6 changed files with 37 additions and 42 deletions

View File

@ -22,6 +22,7 @@ package org.elasticsearch.index.mapper.internal;
import org.apache.lucene.document.Field;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.search.Query;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.common.settings.Settings;
@ -32,6 +33,7 @@ import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperParsingException;
import org.elasticsearch.index.mapper.MetadataFieldMapper;
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.query.QueryShardContext;
import java.io.IOException;
import java.util.ArrayList;
@ -192,6 +194,14 @@ public class FieldNamesFieldMapper extends MetadataFieldMapper {
public boolean useTermQueryWithQueryString() {
return true;
}
@Override
public Query termQuery(Object value, QueryShardContext context) {
if (isEnabled() == false) {
throw new IllegalStateException("Cannot run [exists] queries if the [_field_names] field is disabled");
}
return super.termQuery(value, context);
}
}
private FieldNamesFieldMapper(Settings indexSettings, MappedFieldType existing) {

View File

@ -23,18 +23,16 @@ import org.apache.lucene.search.BooleanClause;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermRangeQuery;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.Objects;
/**
@ -82,38 +80,18 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder<ExistsQueryBuilder>
return Queries.newMatchNoDocsQuery();
}
ObjectMapper objectMapper = context.getObjectMapper(fieldPattern);
if (objectMapper != null) {
// automatic make the object mapper pattern
fieldPattern = fieldPattern + ".*";
}
Collection<String> fields = context.simpleMatchToIndexNames(fieldPattern);
if (fields.isEmpty()) {
// no fields exists, so we should not match anything
return Queries.newMatchNoDocsQuery();
final Collection<String> fields;
if (context.getObjectMapper(fieldPattern) != null) {
// the _field_names field also indexes objects, so we don't have to
// do any more work to support exists queries on whole objects
fields = Collections.singleton(fieldPattern);
} else {
fields = context.simpleMatchToIndexNames(fieldPattern);
}
BooleanQuery.Builder boolFilterBuilder = new BooleanQuery.Builder();
for (String field : fields) {
MappedFieldType fieldType = context.fieldMapper(field);
Query filter = null;
if (fieldNamesFieldType.isEnabled()) {
final String f;
if (fieldType != null) {
f = fieldType.name();
} else {
f = field;
}
filter = fieldNamesFieldType.termQuery(f, context);
}
// if _field_names are not indexed, we need to go the slow way
if (filter == null && fieldType != null) {
filter = fieldType.rangeQuery(null, null, true, true);
}
if (filter == null) {
filter = new TermRangeQuery(field, null, null, true, true);
}
Query filter = fieldNamesFieldType.termQuery(field, context);
boolFilterBuilder.add(filter, BooleanClause.Occur.SHOULD);
}
return new ConstantScoreQuery(boolFilterBuilder.build());

View File

@ -18,6 +18,9 @@
*/
package org.elasticsearch.index.mapper.internal;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.elasticsearch.index.mapper.FieldTypeTestCase;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.junit.Before;
@ -38,4 +41,15 @@ public class FieldNamesFieldTypeTests extends FieldTypeTestCase {
}
});
}
public void testTermQuery() {
FieldNamesFieldMapper.FieldNamesFieldType type = new FieldNamesFieldMapper.FieldNamesFieldType();
type.setName(FieldNamesFieldMapper.CONTENT_TYPE);
type.setEnabled(true);
Query termQuery = type.termQuery("field_name", null);
assertEquals(new TermQuery(new Term(FieldNamesFieldMapper.CONTENT_TYPE, "field_name")), termQuery);
type.setEnabled(false);
IllegalStateException e = expectThrows(IllegalStateException.class, () -> type.termQuery("field_name", null));
assertEquals("Cannot run [exists] queries if the [_field_names] field is disabled", e.getMessage());
}
}

View File

@ -24,7 +24,6 @@ import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.Query;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import java.io.IOException;
import java.util.Collection;
@ -55,13 +54,8 @@ public class ExistsQueryBuilderTests extends AbstractQueryTestCase<ExistsQueryBu
@Override
protected void doAssertLuceneQuery(ExistsQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
String fieldPattern = queryBuilder.fieldName();
ObjectMapper objectMapper = context.getObjectMapper(fieldPattern);
if (objectMapper != null) {
// automatic make the object mapper pattern
fieldPattern = fieldPattern + ".*";
}
Collection<String> fields = context.simpleMatchToIndexNames(fieldPattern);
if (getCurrentTypes().length == 0 || fields.size() == 0) {
if (getCurrentTypes().length == 0) {
assertThat(query, instanceOf(BooleanQuery.class));
BooleanQuery booleanQuery = (BooleanQuery) query;
assertThat(booleanQuery.clauses().size(), equalTo(0));

View File

@ -58,9 +58,6 @@ public class ExistsIT extends ESIntegTestCase {
XContentBuilder mapping = XContentBuilder.builder(JsonXContent.jsonXContent)
.startObject()
.startObject("type")
.startObject(FieldNamesFieldMapper.NAME)
.field("enabled", randomBoolean())
.endObject()
.startObject("properties")
.startObject("foo")
.field("type", "text")
@ -89,10 +86,10 @@ public class ExistsIT extends ESIntegTestCase {
.endObject();
assertAcked(client().admin().indices().prepareCreate("idx").addMapping("type", mapping));
@SuppressWarnings("unchecked")
Map<String, Object> barObject = new HashMap<>();
barObject.put("foo", "bar");
barObject.put("bar", singletonMap("bar", "foo"));
@SuppressWarnings("unchecked")
final Map<String, Object>[] sources = new Map[] {
// simple property
singletonMap("foo", "bar"),

View File

@ -122,6 +122,8 @@ in favour of `query` and `no_match_query`.
upper limit is needed then the `max_children` parameter shouldn't be specified
at all.
* The `exists` query will now fail if the `_field_names` field is disabled.
==== Top level `filter` parameter