From 38a76bf79ffa7a88c99b70c908f3d8e190e9cb66 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Wed, 1 Mar 2017 15:30:50 +0100 Subject: [PATCH 1/2] Fix query_string_query to transform "foo:*" in an exists query on the field name 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 --- .../classic/ExistsFieldQueryExtension.java | 3 +- .../classic/MapperQueryParser.java | 28 +++++++++--------- .../index/query/ExistsQueryBuilder.java | 5 ++++ .../query/QueryStringQueryBuilderTests.java | 29 ++++++++++++++++++- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java b/core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java index 7c3e8652c072d..e1411949c5fd9 100644 --- a/core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java +++ b/core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java @@ -19,7 +19,6 @@ package org.apache.lucene.queryparser.classic; -import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.Query; import org.elasticsearch.index.query.ExistsQueryBuilder; import org.elasticsearch.index.query.QueryShardContext; @@ -30,6 +29,6 @@ public class ExistsFieldQueryExtension implements FieldQueryExtension { @Override public Query query(QueryShardContext context, String queryText) { - return new ConstantScoreQuery(ExistsQueryBuilder.newFilter(context, queryText)); + return ExistsQueryBuilder.newFilter(context, queryText); } } diff --git a/core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java b/core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java index 2b2576b50b694..22ecf37c3a66e 100644 --- a/core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java +++ b/core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java @@ -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 fields = extractMultiFields(field); if (fields != null) { @@ -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(); diff --git a/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java index 07d652cb3337c..217a951918f45 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java @@ -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); diff --git a/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java index 9d659b42573c7..b3cdae4141752 100644 --- a/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -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; @@ -806,8 +807,34 @@ 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 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 testFromJson() throws IOException { From 86d06973f3690332f71246eff95b429a6da69291 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Wed, 1 Mar 2017 17:48:15 +0100 Subject: [PATCH 2/2] Switch to wildcard query when _field_names is disabled --- .../classic/ExistsFieldQueryExtension.java | 10 ++++++++ .../mapping/put/PutMappingRequest.java | 2 +- .../index/query/ExistsQueryBuilder.java | 2 +- .../query/QueryStringQueryBuilderTests.java | 25 +++++++++++++++++++ 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java b/core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java index e1411949c5fd9..b74dbb1184d24 100644 --- a/core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java +++ b/core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java @@ -19,7 +19,10 @@ package org.apache.lucene.queryparser.classic; +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; @@ -29,6 +32,13 @@ public class ExistsFieldQueryExtension implements FieldQueryExtension { @Override public Query query(QueryShardContext context, String 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); } } diff --git a/core/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java b/core/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java index b0e7056ef1d4e..dfd50cf8cfd44 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java +++ b/core/src/main/java/org/elasticsearch/action/admin/indices/mapping/put/PutMappingRequest.java @@ -61,7 +61,7 @@ public class PutMappingRequest extends AcknowledgedRequest im private static ObjectHashSet RESERVED_FIELDS = ObjectHashSet.from( "_uid", "_id", "_type", "_source", "_all", "_analyzer", "_parent", "_routing", "_index", - "_size", "_timestamp", "_ttl" + "_size", "_timestamp", "_ttl", "_field_names" ); private String[] indices; diff --git a/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java index 217a951918f45..f8e3bdec0d96e 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java @@ -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."); diff --git a/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java index b3cdae4141752..617e89531cc49 100644 --- a/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -46,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; @@ -837,6 +840,28 @@ public void testExistsFieldQuery() throws Exception { 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" +