Skip to content

Commit

Permalink
Fix query_string_query to transform "foo:*" in an exists query on the…
Browse files Browse the repository at this point in the history 
… field name (#23433)

Currently "foo:*" is parsed as prefix query on the field `foo` unless the field is defined in `default_field` or `fields`.
This commit fixes this behavior, "foo:*" is now rewritten to an exists query on the field name.
This change also removes the assumption that "_all:*" should return all docs.

relates #23356
  • Loading branch information
@jimczi
jimczi committed Mar 2, 2017
1 parent cc65a94 commit 6519e12
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 19 deletions.
13 changes: 11 additions & 2 deletions core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java
View file
Expand Up @@ -19,8 +19,10 @@

package org.apache.lucene.queryparser.classic;

import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.WildcardQuery;
import org.elasticsearch.index.mapper.FieldNamesFieldMapper;
import org.elasticsearch.index.query.ExistsQueryBuilder;
import org.elasticsearch.index.query.QueryShardContext;

Expand All @@ -30,6 +32,13 @@ public class ExistsFieldQueryExtension implements FieldQueryExtension {

@Override
public Query query(QueryShardContext context, String queryText) {
return new ConstantScoreQuery(ExistsQueryBuilder.newFilter(context, queryText));
final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType =
(FieldNamesFieldMapper.FieldNamesFieldType) context.getMapperService().fullName(FieldNamesFieldMapper.NAME);
if (fieldNamesFieldType.isEnabled() == false) {
// The field_names_field is disabled so we switch to a wildcard query that matches all terms
return new WildcardQuery(new Term(queryText, "*"));
}

return ExistsQueryBuilder.newFilter(context, queryText);
}
}
28 changes: 13 additions & 15 deletions core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java
View file
Expand Up @@ -567,22 +567,16 @@ private Query getPossiblyAnalyzedPrefixQuery(String field, String termStr) throw

@Override
protected Query getWildcardQuery(String field, String termStr) throws ParseException {
if (termStr.equals("*")) {
// we want to optimize for match all query for the "*:*", and "*" cases
if ("*".equals(field) || Objects.equals(field, this.field)) {
String actualField = field;
if (actualField == null) {
actualField = this.field;
}
if (actualField == null) {
return newMatchAllDocsQuery();
}
if ("*".equals(actualField) || "_all".equals(actualField)) {
return newMatchAllDocsQuery();
}
// effectively, we check if a field exists or not
return FIELD_QUERY_EXTENSIONS.get(ExistsFieldQueryExtension.NAME).query(context, actualField);
if (termStr.equals("*") && field != null) {
if ("*".equals(field)) {
return newMatchAllDocsQuery();
}
String actualField = field;
if (actualField == null) {
actualField = this.field;
}
// effectively, we check if a field exists or not
return FIELD_QUERY_EXTENSIONS.get(ExistsFieldQueryExtension.NAME).query(context, actualField);
}
Collection<String> fields = extractMultiFields(field);
if (fields != null) {
Expand Down Expand Up @@ -620,6 +614,10 @@ protected Query getWildcardQuery(String field, String termStr) throws ParseExcep
}

private Query getWildcardQuerySingle(String field, String termStr) throws ParseException {
if ("*".equals(termStr)) {
// effectively, we check if a field exists or not
return FIELD_QUERY_EXTENSIONS.get(ExistsFieldQueryExtension.NAME).query(context, field);
}
String indexedNameField = field;
currentFieldType = null;
Analyzer oldAnalyzer = getAnalyzer();
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java
View file
Expand Up @@ -61,7 +61,7 @@ public class PutMappingRequest extends AcknowledgedRequest<PutMappingRequest> im

private static ObjectHashSet<String> RESERVED_FIELDS = ObjectHashSet.from(
"_uid", "_id", "_type", "_source", "_all", "_analyzer", "_parent", "_routing", "_index",
"_size", "_timestamp", "_ttl"
"_size", "_timestamp", "_ttl", "_field_names"
);

private String[] indices;
Expand Down
7 changes: 6 additions & 1 deletion core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java
View file
Expand Up @@ -129,7 +129,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException {

public static Query newFilter(QueryShardContext context, String fieldPattern) {
final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType =
(FieldNamesFieldMapper.FieldNamesFieldType)context.getMapperService().fullName(FieldNamesFieldMapper.NAME);
(FieldNamesFieldMapper.FieldNamesFieldType) context.getMapperService().fullName(FieldNamesFieldMapper.NAME);
if (fieldNamesFieldType == null) {
// can only happen when no types exist, so no docs exist either
return Queries.newMatchNoDocsQuery("Missing types in \"" + NAME + "\" query.");
Expand All @@ -144,6 +144,11 @@ public static Query newFilter(QueryShardContext context, String fieldPattern) {
fields = context.simpleMatchToIndexNames(fieldPattern);
}

if (fields.size() == 1) {
Query filter = fieldNamesFieldType.termQuery(fields.iterator().next(), context);
return new ConstantScoreQuery(filter);
}

BooleanQuery.Builder boolFilterBuilder = new BooleanQuery.Builder();
for (String field : fields) {
Query filter = fieldNamesFieldType.termQuery(field, context);
Expand Down
52 changes: 52 additions & 0 deletions core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java
View file
Expand Up @@ -27,6 +27,7 @@
import org.apache.lucene.search.BooleanClause.Occur;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.BoostQuery;
import org.apache.lucene.search.ConstantScoreQuery;
import org.apache.lucene.search.DisjunctionMaxQuery;
import org.apache.lucene.search.FuzzyQuery;
import org.apache.lucene.search.MatchAllDocsQuery;
Expand All @@ -45,11 +46,14 @@
import org.apache.lucene.search.spans.SpanTermQuery;
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.lucene.all.AllTermQuery;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.AbstractQueryTestCase;
import org.hamcrest.Matchers;
Expand Down Expand Up @@ -806,10 +810,58 @@ public void testToQuerySplitOnWhitespace() throws IOException {
.build();
assertThat(query, equalTo(expectedQuery));
}
}

public void testExistsFieldQuery() throws Exception {
assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
QueryShardContext context = createShardContext();
QueryStringQueryBuilder queryBuilder = new QueryStringQueryBuilder("foo:*");
Query query = queryBuilder.toQuery(context);
Query expected = new ConstantScoreQuery(new TermQuery(new Term("_field_names", "foo")));
assertThat(query, equalTo(expected));

queryBuilder = new QueryStringQueryBuilder("_all:*");
query = queryBuilder.toQuery(context);
expected = new ConstantScoreQuery(new TermQuery(new Term("_field_names", "_all")));
assertThat(query, equalTo(expected));

queryBuilder = new QueryStringQueryBuilder("*:*");
query = queryBuilder.toQuery(context);
expected = new MatchAllDocsQuery();
assertThat(query, equalTo(expected));

queryBuilder = new QueryStringQueryBuilder("*");
query = queryBuilder.toQuery(context);
List<Query> fieldQueries = new ArrayList<> ();
for (String type : QueryStringQueryBuilder.allQueryableDefaultFields(context).keySet()) {
fieldQueries.add(new ConstantScoreQuery(new TermQuery(new Term("_field_names", type))));
}
expected = new DisjunctionMaxQuery(fieldQueries, 0f);
assertThat(query, equalTo(expected));
}

public void testDisabledFieldNamesField() throws Exception {
QueryShardContext context = createShardContext();
context.getMapperService().merge("new_type",
new CompressedXContent(
PutMappingRequest.buildFromSimplifiedDef("new_type",
"foo", "type=text",
"_field_names", "enabled=false").string()),
MapperService.MergeReason.MAPPING_UPDATE, true);
QueryStringQueryBuilder queryBuilder = new QueryStringQueryBuilder("foo:*");
Query query = queryBuilder.toQuery(context);
Query expected = new WildcardQuery(new Term("foo", "*"));
assertThat(query, equalTo(expected));
context.getMapperService().merge("new_type",
new CompressedXContent(
PutMappingRequest.buildFromSimplifiedDef("new_type",
"foo", "type=text",
"_field_names", "enabled=true").string()),
MapperService.MergeReason.MAPPING_UPDATE, true);
}



public void testFromJson() throws IOException {
String json =
"{\n" +
Expand Down

0 comments on commit 6519e12

Please sign in to comment.