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

Add "all fields" execution mode to simple_query_string query #21341

Merged
merged 1 commit into from
Nov 9, 2016
Merged

Add "all fields" execution mode to simple_query_string query #21341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions core/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public class QueryStringQueryBuilder extends AbstractQueryBuilder<QueryStringQue
private static final ParseField ALL_FIELDS_FIELD = new ParseField("all_fields");

// Mapping types the "all-ish" query can be executed against
private static final Set<String> ALLOWED_QUERY_MAPPER_TYPES;
public static final Set<String> ALLOWED_QUERY_MAPPER_TYPES;

static {
ALLOWED_QUERY_MAPPER_TYPES = new HashSet<>();
Expand Down Expand Up @@ -908,7 +908,11 @@ protected int doHashCode() {
timeZone == null ? 0 : timeZone.getID(), escape, maxDeterminizedStates, splitOnWhitespace, useAllFields);
}

private Map<String, Float> allQueryableDefaultFields(QueryShardContext context) {
/**
* Given a shard context, return a map of all fields in the mappings that
* can be queried. The map will be field name to a float of 1.0f.
*/
public static Map<String, Float> allQueryableDefaultFields(QueryShardContext context) {
Collection<String> allFields = context.simpleMatchToIndexNames("*");
Map<String, Float> fields = new HashMap<>();
for (String fieldName : allFields) {
Expand Down Expand Up @@ -943,6 +947,10 @@ protected Query doToQuery(QueryShardContext context) throws IOException {

Map<String, Float> resolvedFields = new TreeMap<>();

if ((useAllFields != null && useAllFields) && (fieldsAndWeights.size() != 0 || this.defaultField != null)) {
throw addValidationError("cannot use [all_fields] parameter in conjunction with [default_field] or [fields]", null);
}

// If explicitly required to use all fields, use all fields, OR:
// Automatically determine the fields (to replace the _all field) if all of the following are true:
// - The _all field is disabled,
Expand Down
6 changes: 6 additions & 0 deletions core/src/main/java/org/elasticsearch/index/query/SimpleQueryParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,12 @@ static class Settings {
public Settings() {
}

public Settings(Settings other) {
this.lenient = other.lenient;
this.analyzeWildcard = other.analyzeWildcard;
this.quoteFieldSuffix = other.quoteFieldSuffix;
}

/** Specifies whether to use lenient parsing, defaults to false. */
public void lenient(boolean lenient) {
this.lenient = lenient;
Expand Down
73 changes: 60 additions & 13 deletions core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQuerySt
private static final ParseField QUERY_FIELD = new ParseField("query");
private static final ParseField FIELDS_FIELD = new ParseField("fields");
private static final ParseField QUOTE_FIELD_SUFFIX_FIELD = new ParseField("quote_field_suffix");
private static final ParseField ALL_FIELDS_FIELD = new ParseField("all_fields");

/** Query text to parse. */
private final String queryText;
Expand All @@ -126,6 +127,8 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQuerySt
private String minimumShouldMatch;
/** Any search flags to be used, ALL by default. */
private int flags = DEFAULT_FLAGS;
/** Flag specifying whether query should be forced to expand to all searchable fields */
private Boolean useAllFields;

/** Further search settings needed by the ES specific query string parser only. */
private Settings settings = new Settings();
Expand Down Expand Up @@ -166,6 +169,7 @@ public SimpleQueryStringBuilder(StreamInput in) throws IOException {
minimumShouldMatch = in.readOptionalString();
if (in.getVersion().onOrAfter(V_5_1_0_UNRELEASED)) {
settings.quoteFieldSuffix(in.readOptionalString());
useAllFields = in.readOptionalBoolean();
}
}

Expand All @@ -191,6 +195,7 @@ protected void doWriteTo(StreamOutput out) throws IOException {
out.writeOptionalString(minimumShouldMatch);
if (out.getVersion().onOrAfter(V_5_1_0_UNRELEASED)) {
out.writeOptionalString(settings.quoteFieldSuffix());
out.writeOptionalBoolean(useAllFields);
}
}

Expand Down Expand Up @@ -240,6 +245,15 @@ public String analyzer() {
return this.analyzer;
}

public Boolean useAllFields() {
return useAllFields;
}

public SimpleQueryStringBuilder useAllFields(Boolean useAllFields) {
this.useAllFields = useAllFields;
return this;
}

/**
* Specify the default operator for the query. Defaults to "OR" if no
* operator is specified.
Expand Down Expand Up @@ -341,17 +355,37 @@ public String minimumShouldMatch() {
protected Query doToQuery(QueryShardContext context) throws IOException {
// field names in builder can have wildcards etc, need to resolve them here
Map<String, Float> resolvedFieldsAndWeights = new TreeMap<>();
// Use the default field if no fields specified
if (fieldsAndWeights.isEmpty()) {
resolvedFieldsAndWeights.put(resolveIndexName(context.defaultField(), context), AbstractQueryBuilder.DEFAULT_BOOST);

if ((useAllFields != null && useAllFields) && (fieldsAndWeights.size() != 0)) {
throw addValidationError("cannot use [all_fields] parameter in conjunction with [fields]", null);
}

// If explicitly required to use all fields, use all fields, OR:
// Automatically determine the fields (to replace the _all field) if all of the following are true:
// - The _all field is disabled,
// - and the default_field has not been changed in the settings
// - and no fields are specified in the request
Settings newSettings = new Settings(settings);
if ((this.useAllFields != null && this.useAllFields) ||
(context.getMapperService().allEnabled() == false &&
"_all".equals(context.defaultField()) &&
this.fieldsAndWeights.isEmpty())) {
resolvedFieldsAndWeights = QueryStringQueryBuilder.allQueryableDefaultFields(context);
// Need to use lenient mode when using "all-mode" so exceptions aren't thrown due to mismatched types
newSettings.lenient(true);
} else {
for (Map.Entry<String, Float> fieldEntry : fieldsAndWeights.entrySet()) {
if (Regex.isSimpleMatchPattern(fieldEntry.getKey())) {
for (String fieldName : context.getMapperService().simpleMatchToIndexNames(fieldEntry.getKey())) {
resolvedFieldsAndWeights.put(fieldName, fieldEntry.getValue());
// Use the default field if no fields specified
if (fieldsAndWeights.isEmpty()) {
resolvedFieldsAndWeights.put(resolveIndexName(context.defaultField(), context), AbstractQueryBuilder.DEFAULT_BOOST);
} else {
for (Map.Entry<String, Float> fieldEntry : fieldsAndWeights.entrySet()) {
if (Regex.isSimpleMatchPattern(fieldEntry.getKey())) {
for (String fieldName : context.getMapperService().simpleMatchToIndexNames(fieldEntry.getKey())) {
resolvedFieldsAndWeights.put(fieldName, fieldEntry.getValue());
}
} else {
resolvedFieldsAndWeights.put(resolveIndexName(fieldEntry.getKey(), context), fieldEntry.getValue());
}
} else {
resolvedFieldsAndWeights.put(resolveIndexName(fieldEntry.getKey(), context), fieldEntry.getValue());
}
}
}
Expand All @@ -369,7 +403,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException {

}

SimpleQueryParser sqp = new SimpleQueryParser(luceneAnalyzer, resolvedFieldsAndWeights, flags, settings, context);
SimpleQueryParser sqp = new SimpleQueryParser(luceneAnalyzer, resolvedFieldsAndWeights, flags, newSettings, context);
sqp.setDefaultOperator(defaultOperator.toBooleanClauseOccur());

Query query = sqp.parse(queryText);
Expand Down Expand Up @@ -419,6 +453,9 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
if (minimumShouldMatch != null) {
builder.field(MINIMUM_SHOULD_MATCH_FIELD.getPreferredName(), minimumShouldMatch);
}
if (useAllFields != null) {
builder.field(ALL_FIELDS_FIELD.getPreferredName(), useAllFields);
}

printBoostAndQueryName(builder);
builder.endObject();
Expand All @@ -439,6 +476,7 @@ public static Optional<SimpleQueryStringBuilder> fromXContent(QueryParseContext
boolean lenient = SimpleQueryStringBuilder.DEFAULT_LENIENT;
boolean analyzeWildcard = SimpleQueryStringBuilder.DEFAULT_ANALYZE_WILDCARD;
String quoteFieldSuffix = null;
Boolean useAllFields = null;

XContentParser.Token token;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
Expand Down Expand Up @@ -502,6 +540,8 @@ public static Optional<SimpleQueryStringBuilder> fromXContent(QueryParseContext
minimumShouldMatch = parser.textOrNull();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, QUOTE_FIELD_SUFFIX_FIELD)) {
quoteFieldSuffix = parser.textOrNull();
} else if (parseContext.getParseFieldMatcher().match(currentFieldName, ALL_FIELDS_FIELD)) {
useAllFields = parser.booleanValue();
} else {
throw new ParsingException(parser.getTokenLocation(), "[" + SimpleQueryStringBuilder.NAME +
"] unsupported field [" + parser.currentName() + "]");
Expand All @@ -517,10 +557,16 @@ public static Optional<SimpleQueryStringBuilder> fromXContent(QueryParseContext
throw new ParsingException(parser.getTokenLocation(), "[" + SimpleQueryStringBuilder.NAME + "] query text missing");
}

if ((useAllFields != null && useAllFields) && (fieldsAndWeights.size() != 0)) {
throw new ParsingException(parser.getTokenLocation(),
"cannot use [all_fields] parameter in conjunction with [fields]");
}

SimpleQueryStringBuilder qb = new SimpleQueryStringBuilder(queryBody);
qb.boost(boost).fields(fieldsAndWeights).analyzer(analyzerName).queryName(queryName).minimumShouldMatch(minimumShouldMatch);
qb.flags(flags).defaultOperator(defaultOperator);
qb.lenient(lenient).analyzeWildcard(analyzeWildcard).boost(boost).quoteFieldSuffix(quoteFieldSuffix);
qb.useAllFields(useAllFields);
return Optional.of(qb);
}

Expand All @@ -531,15 +577,16 @@ public String getWriteableName() {

@Override
protected int doHashCode() {
return Objects.hash(fieldsAndWeights, analyzer, defaultOperator, queryText, minimumShouldMatch, settings, flags);
return Objects.hash(fieldsAndWeights, analyzer, defaultOperator, queryText, minimumShouldMatch, settings, flags, useAllFields);
}

@Override
protected boolean doEquals(SimpleQueryStringBuilder other) {
return Objects.equals(fieldsAndWeights, other.fieldsAndWeights) && Objects.equals(analyzer, other.analyzer)
&& Objects.equals(defaultOperator, other.defaultOperator) && Objects.equals(queryText, other.queryText)
&& Objects.equals(minimumShouldMatch, other.minimumShouldMatch)
&& Objects.equals(settings, other.settings) && (flags == other.flags);
&& Objects.equals(settings, other.settings)
&& (flags == other.flags)
&& (useAllFields == other.useAllFields);
}
}

25 changes: 24 additions & 1 deletion core/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.util.TestUtil;
import org.elasticsearch.cluster.metadata.MetaData;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.AbstractQueryTestCase;

Expand All @@ -42,6 +44,7 @@
import java.util.Map;
import java.util.Set;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.instanceOf;
Expand Down Expand Up @@ -252,7 +255,12 @@ protected void doAssertLuceneQuery(SimpleQueryStringBuilder queryBuilder, Query
Map.Entry<String, Float> field = queryBuilder.fields().entrySet().iterator().next();
assertTermOrBoostQuery(query, field.getKey(), queryBuilder.value(), field.getValue());
} else if (queryBuilder.fields().size() == 0) {
assertTermQuery(query, MetaData.ALL, queryBuilder.value());
MapperService ms = context.mapperService();
if (ms.allEnabled()) {
assertTermQuery(query, MetaData.ALL, queryBuilder.value());
} else {
assertThat(query.getClass(), equalTo(MatchNoDocsQuery.class));
}
} else {
fail("Encountered lucene query type we do not have a validation implementation for in our "
+ SimpleQueryStringBuilderTests.class.getSimpleName());
Expand Down Expand Up @@ -398,4 +406,19 @@ public void testExpandedTerms() throws Exception {
expected = new FuzzyQuery(new Term(STRING_FIELD_NAME, "abc"), 1);
assertEquals(expected, query);
}

public void testAllFieldsWithFields() throws IOException {
String json =
"{\n" +
" \"simple_query_string\" : {\n" +
" \"query\" : \"this that thus\",\n" +
" \"fields\" : [\"foo\"],\n" +
" \"all_fields\" : true\n" +
" }\n" +
"}";

ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(json));
assertThat(e.getMessage(),
containsString("cannot use [all_fields] parameter in conjunction with [fields]"));
}
}
47 changes: 26 additions & 21 deletions core/src/test/java/org/elasticsearch/search/query/QueryStringIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@
package org.elasticsearch.search.query;

import org.apache.lucene.util.LuceneTestCase;
import org.elasticsearch.ExceptionsHelper;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.index.query.Operator;
import org.elasticsearch.index.query.QueryStringQueryBuilder;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
Expand Down Expand Up @@ -201,33 +203,36 @@ public void testKeywordWithWhitespace() throws Exception {
assertHits(resp.getHits(), "2", "3");
assertHitCount(resp, 2L);

// Will be fixed once https://github.com/elastic/elasticsearch/pull/20965 is in
// resp = client().prepareSearch("test")
// .setQuery(queryStringQuery("Foo Bar").splitOnWhitespcae(false))
// .get();
// assertHits(resp.getHits(), "1", "2", "3");
// assertHitCount(resp, 3L);
resp = client().prepareSearch("test")
.setQuery(queryStringQuery("Foo Bar").splitOnWhitespace(false))
.get();
assertHits(resp.getHits(), "1", "2", "3");
assertHitCount(resp, 3L);
}

public void testExplicitAllFieldsRequested() throws Exception {
String indexBody = copyToStringFromClasspath("/org/elasticsearch/search/query/all-query-index-with-all.json");
prepareCreate("test2").setSource(indexBody).get();
ensureGreen("test2");

List<IndexRequestBuilder> reqs = new ArrayList<>();
reqs.add(client().prepareIndex("test", "doc", "1").setSource("f1", "foo",
"f_date", "2015/09/02",
"f_float", "1.7",
"f_ip", "127.0.0.1"));
reqs.add(client().prepareIndex("test", "doc", "2").setSource("f1", "bar",
"f_date", "2015/09/01",
"f_float", "1.8",
"f_ip", "127.0.0.2"));
reqs.add(client().prepareIndex("test2", "doc", "1").setSource("f1", "foo", "f2", "eggplant"));
indexRandom(true, false, reqs);

SearchResponse resp = client().prepareSearch("test").setQuery(
queryStringQuery("127.0.0.2 \"2015/09/02\"")
.field("f_ip") // Usually this would mean we wouldn't search "all" fields
.useAllFields(true)) // ... unless explicitly requested
.get();
assertHits(resp.getHits(), "1", "2");
assertHitCount(resp, 2L);
SearchResponse resp = client().prepareSearch("test2").setQuery(
queryStringQuery("foo eggplent").defaultOperator(Operator.AND)).get();
assertHitCount(resp, 0L);

resp = client().prepareSearch("test2").setQuery(
queryStringQuery("foo eggplent").defaultOperator(Operator.AND).useAllFields(true)).get();
assertHits(resp.getHits(), "1");
assertHitCount(resp, 1L);

Exception e = expectThrows(Exception.class, () ->
client().prepareSearch("test2").setQuery(
queryStringQuery("blah").field("f1").useAllFields(true)).get());
assertThat(ExceptionsHelper.detailedMessage(e),
containsString("cannot use [all_fields] parameter in conjunction with [default_field] or [fields]"));
}

@LuceneTestCase.AwaitsFix(bugUrl="currently can't perform phrase queries on fields that don't support positions")
Expand Down
Loading