Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix query_string_query to transform "foo:*" in an exists query on the field name #23433

Merged
merged 2 commits into from Mar 2, 2017
Merged

Fix query_string_query to transform "foo:*" in an exists query on the field name #23433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38a76bf
Fix query_string_query to transform "foo:*" in an exists query on the…
jimczi Mar 1, 2017
86d0697
Switch to wildcard query when _field_names is disabled
jimczi Mar 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 11 additions & 2 deletions core/src/main/java/org/apache/lucene/queryparser/classic/ExistsFieldQueryExtension.java
View file
Open in desktop
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, "*"));
Copy link
Contributor

@jpountz jpountz Mar 1, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that will only work on text/keyword fields? I think we need to perform this check up-front in the MappedFieldType?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True but that's also the current behavior. foo:* does not work if foo is not a text/keyword field. I don't get what you mean by check up-front ? Should we add another function in the MappedFieldType that returns match all query for the field ? Numbers would return open ranges query, text/keyword wildcard query and we would use this function only if the _field_names field is disabled ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sorry I thought we already had such a method, hence my comment! I think things are good this way, wildcard queries only make sense on text/keyword anyway. Thanks for the explanation!

}

return ExistsQueryBuilder.newFilter(context, queryText);
}
}
28 changes: 13 additions & 15 deletions core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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