Skip to content

Commit

Permalink
Allow fields to be set to * (#42301)
Browse files Browse the repository at this point in the history
Allow for SimpleQueryString, QueryString and MultiMatchQuery
to set the `fields` parameter to the wildcard `*`. If so, set
the leniency to `true`, to achieve the same behaviour as from the
`"default_field" : "*" setting.

Furthermore,  check if `*` is in the list of the `default_field` but
not necessarily as the 1st element.

Closes: #39577
(cherry picked from commit e75ff0c)
  • Loading branch information
matriv committed May 23, 2019
1 parent fa98cbe commit 0777223
Show file tree
Hide file tree
Showing 7 changed files with 227 additions and 79 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.unit.Fuzziness;
import org.elasticsearch.common.xcontent.DeprecationHandler;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
Expand Down Expand Up @@ -803,18 +802,20 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
multiMatchQuery.setTranspositions(fuzzyTranspositions);

Map<String, Float> newFieldsBoosts;
boolean isAllField;
if (fieldsBoosts.isEmpty()) {
// no fields provided, defaults to index.query.default_field
List<String> defaultFields = context.defaultFields();
boolean isAllField = defaultFields.size() == 1 && Regex.isMatchAllPattern(defaultFields.get(0));
if (isAllField && lenient == null) {
// Sets leniency to true if not explicitly
// set in the request
multiMatchQuery.setLenient(true);
}
newFieldsBoosts = QueryParserHelper.resolveMappingFields(context, QueryParserHelper.parseFieldsAndWeights(defaultFields));
isAllField = QueryParserHelper.hasAllFieldsWildcard(defaultFields);
} else {
newFieldsBoosts = QueryParserHelper.resolveMappingFields(context, fieldsBoosts);
isAllField = QueryParserHelper.hasAllFieldsWildcard(fieldsBoosts.keySet());
}
if (isAllField && lenient == null) {
// Sets leniency to true if not explicitly
// set in the request
multiMatchQuery.setLenient(true);
}
return multiMatchQuery.parse(type, newFieldsBoosts, value, minimumShouldMatch);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -852,11 +852,14 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
}
} else if (fieldsAndWeights.size() > 0) {
final Map<String, Float> resolvedFields = QueryParserHelper.resolveMappingFields(context, fieldsAndWeights);
queryParser = new QueryStringQueryParser(context, resolvedFields, isLenient);
if (QueryParserHelper.hasAllFieldsWildcard(fieldsAndWeights.keySet())) {
queryParser = new QueryStringQueryParser(context, resolvedFields, lenient == null ? true : lenient);
} else {
queryParser = new QueryStringQueryParser(context, resolvedFields, isLenient);
}
} else {
List<String> defaultFields = context.defaultFields();
boolean isAllField = defaultFields.size() == 1 && Regex.isMatchAllPattern(defaultFields.get(0));
if (isAllField) {
if (QueryParserHelper.hasAllFieldsWildcard(defaultFields)) {
queryParser = new QueryStringQueryParser(context, lenient == null ? true : lenient);
} else {
final Map<String, Float> resolvedFields = QueryParserHelper.resolveMappingFields(context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.search.QueryParserHelper;
Expand Down Expand Up @@ -404,16 +403,19 @@ public SimpleQueryStringBuilder fuzzyTranspositions(boolean fuzzyTranspositions)
protected Query doToQuery(QueryShardContext context) throws IOException {
Settings newSettings = new Settings(settings);
final Map<String, Float> resolvedFieldsAndWeights;
boolean isAllField;
if (fieldsAndWeights.isEmpty() == false) {
resolvedFieldsAndWeights = QueryParserHelper.resolveMappingFields(context, fieldsAndWeights);
isAllField = QueryParserHelper.hasAllFieldsWildcard(fieldsAndWeights.keySet());
} else {
List<String> defaultFields = context.defaultFields();
boolean isAllField = defaultFields.size() == 1 && Regex.isMatchAllPattern(defaultFields.get(0));
if (isAllField) {
newSettings.lenient(lenientSet ? settings.lenient() : true);
}
resolvedFieldsAndWeights = QueryParserHelper.resolveMappingFields(context,
QueryParserHelper.parseFieldsAndWeights(defaultFields));
isAllField = QueryParserHelper.hasAllFieldsWildcard(defaultFields);
}

if (isAllField) {
newSettings.lenient(lenientSet ? settings.lenient() : true);
}

final SimpleQueryStringQueryParser sqp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,4 +161,12 @@ private static void checkForTooManyFields(Map<String, Float> fields, QueryShardC
throw new IllegalArgumentException("field expansion matches too many fields, limit: " + limit + ", got: " + fields.size());
}
}

/**
* Returns true if any of the fields is the wildcard {@code *}, false otherwise.
* @param fields A collection of field names
*/
public static boolean hasAllFieldsWildcard(Collection<String> fields) {
return fields.stream().anyMatch(Regex::isMatchAllPattern);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import static org.hamcrest.CoreMatchers.anyOf;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.hasItems;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.collection.IsCollectionWithSize.hasSize;

Expand Down Expand Up @@ -409,52 +410,79 @@ public void testToFuzzyQuery() throws Exception {
public void testDefaultField() throws Exception {
QueryShardContext context = createShardContext();
MultiMatchQueryBuilder builder = new MultiMatchQueryBuilder("hello");
// should pass because we set lenient to true when default field is `*`
// default value `*` sets leniency to true
Query query = builder.toQuery(context);
assertThat(query, instanceOf(DisjunctionMaxQuery.class));

context.getIndexSettings().updateIndexMetaData(
newIndexMeta("index", context.getIndexSettings().getSettings(),
Settings.builder().putList("index.query.default_field", STRING_FIELD_NAME, STRING_FIELD_NAME_2 + "^5")
.build())
);

MultiMatchQueryBuilder qb = new MultiMatchQueryBuilder("hello");
query = qb.toQuery(context);
DisjunctionMaxQuery expected = new DisjunctionMaxQuery(
Arrays.asList(
new TermQuery(new Term(STRING_FIELD_NAME, "hello")),
new BoostQuery(new TermQuery(new Term(STRING_FIELD_NAME_2, "hello")), 5.0f)
), 0.0f
);
assertEquals(expected, query);
assertQueryWithAllFieldsWildcard(query);

try {
// `*` is in the list of the default_field => leniency set to true
context.getIndexSettings().updateIndexMetaData(
newIndexMeta("index", context.getIndexSettings().getSettings(), Settings.builder().putList("index.query.default_field",
STRING_FIELD_NAME, "*", STRING_FIELD_NAME_2).build())
);
query = new MultiMatchQueryBuilder("hello")
.toQuery(context);
assertQueryWithAllFieldsWildcard(query);

context.getIndexSettings().updateIndexMetaData(
newIndexMeta("index", context.getIndexSettings().getSettings(),
Settings.builder().putList("index.query.default_field", STRING_FIELD_NAME, STRING_FIELD_NAME_2 + "^5")
.build())
);
MultiMatchQueryBuilder qb = new MultiMatchQueryBuilder("hello");
query = qb.toQuery(context);
DisjunctionMaxQuery expected = new DisjunctionMaxQuery(
Arrays.asList(
new TermQuery(new Term(STRING_FIELD_NAME, "hello")),
new BoostQuery(new TermQuery(new Term(STRING_FIELD_NAME_2, "hello")), 5.0f)
), 0.0f
);
assertEquals(expected, query);

context.getIndexSettings().updateIndexMetaData(
newIndexMeta("index", context.getIndexSettings().getSettings(),
Settings.builder().putList("index.query.default_field",
STRING_FIELD_NAME, STRING_FIELD_NAME_2 + "^5", INT_FIELD_NAME).build())
);
// should fail because lenient defaults to false
IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> qb.toQuery(context));
assertThat(exc, instanceOf(NumberFormatException.class));
assertThat(exc.getMessage(), equalTo("For input string: \"hello\""));

// explicitly sets lenient
qb.lenient(true);
query = qb.toQuery(context);
expected = new DisjunctionMaxQuery(
Arrays.asList(
new TermQuery(new Term(STRING_FIELD_NAME, "hello")),
new BoostQuery(new TermQuery(new Term(STRING_FIELD_NAME_2, "hello")), 5.0f),
new MatchNoDocsQuery("failed [mapped_int] query, caused by number_format_exception:[For input string: \"hello\"]")
), 0.0f
);
assertEquals(expected, query);

} finally {
// Reset to the default value
context.getIndexSettings().updateIndexMetaData(
newIndexMeta("index", context.getIndexSettings().getSettings(),
Settings.builder().putNull("index.query.default_field").build())
);
}
}

context.getIndexSettings().updateIndexMetaData(
newIndexMeta("index", context.getIndexSettings().getSettings(),
Settings.builder().putList("index.query.default_field",
STRING_FIELD_NAME, STRING_FIELD_NAME_2 + "^5", INT_FIELD_NAME).build())
);
// should fail because lenient defaults to false
IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> qb.toQuery(context));
assertThat(exc, instanceOf(NumberFormatException.class));
assertThat(exc.getMessage(), equalTo("For input string: \"hello\""));

// explicitly sets lenient
qb.lenient(true);
query = qb.toQuery(context);
expected = new DisjunctionMaxQuery(
Arrays.asList(
new TermQuery(new Term(STRING_FIELD_NAME, "hello")),
new BoostQuery(new TermQuery(new Term(STRING_FIELD_NAME_2, "hello")), 5.0f),
new MatchNoDocsQuery("failed [mapped_int] query, caused by number_format_exception:[For input string: \"hello\"]")
), 0.0f
);
assertEquals(expected, query);
public void testAllFieldsWildcard() throws Exception {
QueryShardContext context = createShardContext();
Query query = new MultiMatchQueryBuilder("hello")
.field("*")
.toQuery(context);
assertQueryWithAllFieldsWildcard(query);

context.getIndexSettings().updateIndexMetaData(
newIndexMeta("index", context.getIndexSettings().getSettings(),
Settings.builder().putNull("index.query.default_field").build())
);
query = new MultiMatchQueryBuilder("hello")
.field(STRING_FIELD_NAME)
.field("*")
.field(STRING_FIELD_NAME_2)
.toQuery(context);
assertQueryWithAllFieldsWildcard(query);
}

public void testWithStopWords() throws Exception {
Expand Down Expand Up @@ -536,4 +564,18 @@ private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings
.build();
return IndexMetaData.builder(name).settings(build).build();
}

private void assertQueryWithAllFieldsWildcard(Query query) {
assertEquals(DisjunctionMaxQuery.class, query.getClass());
DisjunctionMaxQuery disjunctionMaxQuery = (DisjunctionMaxQuery) query;
int noMatchNoDocsQueries = 0;
for (Query q : disjunctionMaxQuery.getDisjuncts()) {
if (q.getClass() == MatchNoDocsQuery.class) {
noMatchNoDocsQueries++;
}
}
assertEquals(11, noMatchNoDocsQueries);
assertThat(disjunctionMaxQuery.getDisjuncts(), hasItems(new TermQuery(new Term(STRING_FIELD_NAME, "hello")),
new TermQuery(new Term(STRING_FIELD_NAME_2, "hello"))));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBooleanSubQuery;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertDisjunctionSubQuery;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.hasItems;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;

Expand Down Expand Up @@ -1258,12 +1259,27 @@ public void testUnmappedFieldRewriteToMatchNoDocs() throws IOException {

public void testDefaultField() throws Exception {
QueryShardContext context = createShardContext();
context.getIndexSettings().updateIndexMetaData(
newIndexMeta("index", context.getIndexSettings().getSettings(), Settings.builder().putList("index.query.default_field",
STRING_FIELD_NAME, STRING_FIELD_NAME_2 + "^5").build())
);
// default value `*` sets leniency to true
Query query = new QueryStringQueryBuilder("hello")
.toQuery(context);
assertQueryWithAllFieldsWildcard(query);

try {
Query query = new QueryStringQueryBuilder("hello")
// `*` is in the list of the default_field => leniency set to true
context.getIndexSettings().updateIndexMetaData(
newIndexMeta("index", context.getIndexSettings().getSettings(), Settings.builder().putList("index.query.default_field",
STRING_FIELD_NAME, "*", STRING_FIELD_NAME_2).build())
);
query = new QueryStringQueryBuilder("hello")
.toQuery(context);
assertQueryWithAllFieldsWildcard(query);


context.getIndexSettings().updateIndexMetaData(
newIndexMeta("index", context.getIndexSettings().getSettings(), Settings.builder().putList("index.query.default_field",
STRING_FIELD_NAME, STRING_FIELD_NAME_2 + "^5").build())
);
query = new QueryStringQueryBuilder("hello")
.toQuery(context);
Query expected = new DisjunctionMaxQuery(
Arrays.asList(
Expand All @@ -1281,6 +1297,21 @@ public void testDefaultField() throws Exception {
}
}

public void testAllFieldsWildcard() throws Exception {
QueryShardContext context = createShardContext();
Query query = new QueryStringQueryBuilder("hello")
.field("*")
.toQuery(context);
assertQueryWithAllFieldsWildcard(query);

query = new QueryStringQueryBuilder("hello")
.field(STRING_FIELD_NAME)
.field("*")
.field(STRING_FIELD_NAME_2)
.toQuery(context);
assertQueryWithAllFieldsWildcard(query);
}

/**
* the quote analyzer should overwrite any other forced analyzer in quoted parts of the query
*/
Expand Down Expand Up @@ -1516,4 +1547,18 @@ private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings
.build();
return IndexMetaData.builder(name).settings(build).build();
}

private void assertQueryWithAllFieldsWildcard(Query query) {
assertEquals(DisjunctionMaxQuery.class, query.getClass());
DisjunctionMaxQuery disjunctionMaxQuery = (DisjunctionMaxQuery) query;
int noMatchNoDocsQueries = 0;
for (Query q : disjunctionMaxQuery.getDisjuncts()) {
if (q.getClass() == MatchNoDocsQuery.class) {
noMatchNoDocsQueries++;
}
}
assertEquals(11, noMatchNoDocsQueries);
assertThat(disjunctionMaxQuery.getDisjuncts(), hasItems(new TermQuery(new Term(STRING_FIELD_NAME, "hello")),
new TermQuery(new Term(STRING_FIELD_NAME_2, "hello"))));
}
}
Loading

0 comments on commit 0777223

Please sign in to comment.