Query string default field (#24214)

Currently any `query_string` query that use a wildcard field with no matching field is rewritten with the `_all` field.

For instance:
````
#creating test doc
PUT testing/t/1
{
  "test": {
    "field_one": "hello",
    "field_two": "world"
  }
}
#searching abc.* (does not exist) -> hit
GET testing/t/_search
{
  "query": {
    "query_string": {
      "fields": [
        "abc.*"
      ],
      "query": "hello"
    }
  }
}
````

This bug first appeared in 5.0 after the query refactoring and impacts only users that use `_all` as default field.
Indices created in 6.x will not have this problem since `_all` is deactivated in this version.

This change fixes this bug by returning a MatchNoDocsQuery for any term that expand to an empty list of field.
This commit is contained in:
Jim Ferenczi 2017-04-20 22:12:20 +02:00 committed by GitHub
parent 82c678b5c7
commit 525101b64d
3 changed files with 39 additions and 8 deletions

View File

@ -57,7 +57,7 @@ import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Collections;
import static java.util.Collections.unmodifiableMap;
import static org.elasticsearch.common.lucene.search.Queries.fixNegativeQueryIfNeeded;
@ -91,7 +91,8 @@ public class MapperQueryParser extends QueryParser {
public void reset(QueryParserSettings settings) {
this.settings = settings;
if (settings.fieldsAndWeights().isEmpty()) {
if (settings.fieldsAndWeights() == null) {
// this query has no explicit fields to query so we fallback to the default field
this.field = settings.defaultField();
} else if (settings.fieldsAndWeights().size() == 1) {
this.field = settings.fieldsAndWeights().keySet().iterator().next();
@ -148,6 +149,11 @@ public class MapperQueryParser extends QueryParser {
if (fields != null) {
if (fields.size() == 1) {
return getFieldQuerySingle(fields.iterator().next(), queryText, quoted);
} else if (fields.isEmpty()) {
// the requested fields do not match any field in the mapping
// happens for wildcard fields only since we cannot expand to a valid field name
// if there is no match in the mappings.
return new MatchNoDocsQuery("empty fields");
}
if (settings.useDisMax()) {
List<Query> queries = new ArrayList<>();
@ -721,7 +727,7 @@ public class MapperQueryParser extends QueryParser {
}
private Query applyBoost(String field, Query q) {
Float fieldBoost = settings.fieldsAndWeights().get(field);
Float fieldBoost = settings.fieldsAndWeights() == null ? null : settings.fieldsAndWeights().get(field);
if (fieldBoost != null && fieldBoost != 1f) {
return new BoostQuery(q, fieldBoost);
}
@ -780,7 +786,8 @@ public class MapperQueryParser extends QueryParser {
if (field != null) {
fields = context.simpleMatchToIndexNames(field);
} else {
fields = settings.fieldsAndWeights().keySet();
Map<String, Float> fieldsAndWeights = settings.fieldsAndWeights();
fields = fieldsAndWeights == null ? Collections.emptyList() : settings.fieldsAndWeights().keySet();
}
return fields;
}

View File

@ -981,7 +981,11 @@ public class QueryStringQueryBuilder extends AbstractQueryBuilder<QueryStringQue
}
qpSettings.lenient(lenient == null ? context.queryStringLenient() : lenient);
}
if (fieldsAndWeights.isEmpty() == false || resolvedFields.isEmpty() == false) {
// We set the fields and weight only if we have explicit fields to query
// Otherwise we set it to null and fallback to the default field.
qpSettings.fieldsAndWeights(resolvedFields);
}
qpSettings.defaultOperator(defaultOperator.toQueryParserOperator());
if (analyzer == null) {

View File

@ -324,7 +324,6 @@ public class QueryStringQueryBuilderTests extends AbstractQueryTestCase<QueryStr
MapperQueryParser queryParser = new MapperQueryParser(createShardContext());
QueryParserSettings settings = new QueryParserSettings("first foo-bar-foobar* last");
settings.defaultField(STRING_FIELD_NAME);
settings.fieldsAndWeights(Collections.emptyMap());
settings.analyzeWildcard(true);
settings.fuzziness(Fuzziness.AUTO);
settings.rewriteMethod(MultiTermQuery.CONSTANT_SCORE_REWRITE);
@ -352,7 +351,6 @@ public class QueryStringQueryBuilderTests extends AbstractQueryTestCase<QueryStr
MapperQueryParser queryParser = new MapperQueryParser(createShardContext());
QueryParserSettings settings = new QueryParserSettings("first foo-bar-foobar* last");
settings.defaultField(STRING_FIELD_NAME);
settings.fieldsAndWeights(Collections.emptyMap());
settings.analyzeWildcard(true);
settings.fuzziness(Fuzziness.AUTO);
settings.rewriteMethod(MultiTermQuery.CONSTANT_SCORE_REWRITE);
@ -390,7 +388,6 @@ public class QueryStringQueryBuilderTests extends AbstractQueryTestCase<QueryStr
MapperQueryParser queryParser = new MapperQueryParser(createShardContext());
QueryParserSettings settings = new QueryParserSettings("");
settings.defaultField(STRING_FIELD_NAME);
settings.fieldsAndWeights(Collections.emptyMap());
settings.fuzziness(Fuzziness.AUTO);
settings.analyzeWildcard(true);
settings.rewriteMethod(MultiTermQuery.CONSTANT_SCORE_REWRITE);
@ -689,6 +686,29 @@ public class QueryStringQueryBuilderTests extends AbstractQueryTestCase<QueryStr
assertThat(phraseQuery.getTerms().length, equalTo(2));
}
public void testToQueryWildcardNonExistingFields() throws IOException {
assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
QueryStringQueryBuilder queryStringQueryBuilder =
new QueryStringQueryBuilder("foo bar").field("invalid*");
Query query = queryStringQueryBuilder.toQuery(createShardContext());
Query expectedQuery = new BooleanQuery.Builder()
.add(new MatchNoDocsQuery("empty fields"), Occur.SHOULD)
.add(new MatchNoDocsQuery("empty fields"), Occur.SHOULD)
.build();
assertThat(expectedQuery, equalTo(query));
queryStringQueryBuilder =
new QueryStringQueryBuilder("field:foo bar").field("invalid*");
query = queryStringQueryBuilder.toQuery(createShardContext());
expectedQuery = new BooleanQuery.Builder()
.add(new TermQuery(new Term("field", "foo")), Occur.SHOULD)
.add(new MatchNoDocsQuery("empty fields"), Occur.SHOULD)
.build();
assertThat(expectedQuery, equalTo(query));
}
public void testToQuerySplitOnWhitespace() throws IOException {
assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
// splitOnWhitespace=false