From 137ce9ad8702d3af14f00c2aa312f23350691dd3 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 20 Apr 2017 15:50:11 +0200 Subject: [PATCH 1/5] Fix query_string_query with wildcard fields 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 problem by ignoring terms without an explicit field if the requested multi fields are not empty. --- .../queryparser/classic/MapperQueryParser.java | 8 +++++--- .../index/query/QueryStringQueryBuilder.java | 6 +++++- .../index/query/QueryStringQueryBuilderTests.java | 14 +++++++++++--- 3 files changed, 21 insertions(+), 7 deletions(-) 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 79f522e8c1fe5..1aa64216cfed9 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 @@ -91,7 +91,8 @@ public MapperQueryParser(QueryShardContext context) { 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(); @@ -721,7 +722,7 @@ protected Query getBooleanQuery(List clauses) throws ParseExcepti } 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 +781,8 @@ private Collection extractMultiFields(String field) { if (field != null) { fields = context.simpleMatchToIndexNames(field); } else { - fields = settings.fieldsAndWeights().keySet(); + fields = + settings.fieldsAndWeights() == null ? Collections.emptyList() : settings.fieldsAndWeights().keySet(); } return fields; } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java index ca716372571cc..fd6f33e27ba87 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java @@ -981,7 +981,11 @@ protected Query doToQuery(QueryShardContext context) throws IOException { } qpSettings.lenient(lenient == null ? context.queryStringLenient() : lenient); } - qpSettings.fieldsAndWeights(resolvedFields); + 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) { 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 86647233aa3fc..7c18af510f357 100644 --- a/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -324,7 +324,6 @@ public void testToQueryWildcarQuery() throws Exception { 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 void testToQueryWilcardQueryWithSynonyms() throws Exception { 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 void testToQueryWithGraph() throws Exception { 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,17 @@ public void testToQueryPhraseQueryBoostAndSlop() throws IOException { 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); + for (boolean b : new boolean[] {true, false}) { + QueryStringQueryBuilder queryStringQueryBuilder = + new QueryStringQueryBuilder("foo bar").field("invalid*"); + Query query = queryStringQueryBuilder.toQuery(createShardContext()); + assertThat(query, equalTo(new BooleanQuery.Builder().build())); + } + + } + public void testToQuerySplitOnWhitespace() throws IOException { assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0); // splitOnWhitespace=false From 732e75f73d4f52c67316c9f723f0e492a3c0bd3a Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 20 Apr 2017 15:54:17 +0200 Subject: [PATCH 2/5] cleanup ut --- .../lucene/queryparser/classic/MapperQueryParser.java | 2 +- .../index/query/QueryStringQueryBuilderTests.java | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) 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 1aa64216cfed9..0a71f2ec16037 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 @@ -57,7 +57,7 @@ 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; 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 7c18af510f357..6c39f902d47ba 100644 --- a/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -688,13 +688,10 @@ public void testToQueryPhraseQueryBoostAndSlop() throws IOException { public void testToQueryWildcardNonExistingFields() throws IOException { assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0); - for (boolean b : new boolean[] {true, false}) { - QueryStringQueryBuilder queryStringQueryBuilder = - new QueryStringQueryBuilder("foo bar").field("invalid*"); - Query query = queryStringQueryBuilder.toQuery(createShardContext()); - assertThat(query, equalTo(new BooleanQuery.Builder().build())); - } - + QueryStringQueryBuilder queryStringQueryBuilder = + new QueryStringQueryBuilder("foo bar").field("invalid*"); + Query query = queryStringQueryBuilder.toQuery(createShardContext()); + assertThat(query, equalTo(new BooleanQuery.Builder().build())); } public void testToQuerySplitOnWhitespace() throws IOException { From e1ca3faf4119a3af2bd47df562388416c0076471 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 20 Apr 2017 17:02:17 +0200 Subject: [PATCH 3/5] Fix expectations when the requested wildcard do not match any field in the mapping In the previous change each non-fielded term inside a `query_string` query was simply ignored. This leads to weird behavior for mixed queries like `foo AND field:bar`. This change fixes this discrepancy by returning a MatchNoDocsQuery for any term that expand to an empty list of field. --- .../queryparser/classic/MapperQueryParser.java | 15 ++++++--------- .../query/QueryStringQueryBuilderTests.java | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 10 deletions(-) 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 0a71f2ec16037..7ace46388ebf0 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 @@ -25,15 +25,7 @@ import org.apache.lucene.analysis.tokenattributes.CharTermAttribute; import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute; import org.apache.lucene.index.Term; -import org.apache.lucene.search.BooleanClause; -import org.apache.lucene.search.BoostQuery; -import org.apache.lucene.search.DisjunctionMaxQuery; -import org.apache.lucene.search.FuzzyQuery; -import org.apache.lucene.search.MatchNoDocsQuery; -import org.apache.lucene.search.MultiPhraseQuery; -import org.apache.lucene.search.PhraseQuery; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.SynonymQuery; +import org.apache.lucene.search.*; import org.apache.lucene.search.spans.SpanNearQuery; import org.apache.lucene.search.spans.SpanOrQuery; import org.apache.lucene.search.spans.SpanQuery; @@ -149,6 +141,11 @@ public Query getFieldQuery(String field, String queryText, boolean quoted) throw 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 queries = new ArrayList<>(); 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 6c39f902d47ba..5b9990c81cd9e 100644 --- a/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -691,7 +691,22 @@ public void testToQueryWildcardNonExistingFields() throws IOException { QueryStringQueryBuilder queryStringQueryBuilder = new QueryStringQueryBuilder("foo bar").field("invalid*"); Query query = queryStringQueryBuilder.toQuery(createShardContext()); - assertThat(query, equalTo(new BooleanQuery.Builder().build())); + 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 { From 938e77087b2e517ec598b06907e7fa51b03cdc7a Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 20 Apr 2017 17:06:30 +0200 Subject: [PATCH 4/5] checkstyle --- .../lucene/queryparser/classic/MapperQueryParser.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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 7ace46388ebf0..2e498646d1b16 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 @@ -25,7 +25,15 @@ import org.apache.lucene.analysis.tokenattributes.CharTermAttribute; import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute; import org.apache.lucene.index.Term; -import org.apache.lucene.search.*; +import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BoostQuery; +import org.apache.lucene.search.DisjunctionMaxQuery; +import org.apache.lucene.search.FuzzyQuery; +import org.apache.lucene.search.MatchNoDocsQuery; +import org.apache.lucene.search.MultiPhraseQuery; +import org.apache.lucene.search.PhraseQuery; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.SynonymQuery; import org.apache.lucene.search.spans.SpanNearQuery; import org.apache.lucene.search.spans.SpanOrQuery; import org.apache.lucene.search.spans.SpanQuery; From 04823258ae4e3d36bf72e51b025ac2dff7ef21f8 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 20 Apr 2017 22:08:07 +0200 Subject: [PATCH 5/5] apply review --- .../apache/lucene/queryparser/classic/MapperQueryParser.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 2e498646d1b16..015972c568417 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 @@ -786,8 +786,8 @@ private Collection extractMultiFields(String field) { if (field != null) { fields = context.simpleMatchToIndexNames(field); } else { - fields = - settings.fieldsAndWeights() == null ? Collections.emptyList() : settings.fieldsAndWeights().keySet(); + Map fieldsAndWeights = settings.fieldsAndWeights(); + fields = fieldsAndWeights == null ? Collections.emptyList() : settings.fieldsAndWeights().keySet(); } return fields; }