From ff03443ab945ad876fedaabf2b447146b9d6a2f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christoph=20B=C3=BCscher?= <cbuescher@posteo.de>
Date: Wed, 21 Nov 2018 03:49:49 +0100
Subject: [PATCH] Fix problem with MatchNoDocsQuery in disjunction queries
 (#35726)

Queries across multiple fields generate MatchNoDocsQuerys for fields that are
unmapped. In certain situation this can lead to erroneous behaviour,
for example when an umapped field is used in a query_string query across
several fields. If some of the tokens in the query string get eliminated by an
analyzer on the mapped fields, the same token will currently generate
MatchNoDocsQuerys combined into a disjunction, which in turn
leads to no matches in the overall query. Instead we should simply not add
MatchNoDocsQuerys to those disjunctions.

Closes #34708
---
 .../index/search/MatchQuery.java              |  1 +
 .../index/search/MultiMatchQuery.java         |  5 ++++-
 .../query/SimpleQueryStringBuilderTests.java  | 20 +++++++++++++++++++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java b/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java
index 597666a7c60..a5b5a763287 100644
--- a/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java
+++ b/server/src/main/java/org/elasticsearch/index/search/MatchQuery.java
@@ -391,6 +391,7 @@ public class MatchQuery {
          * Checks if graph analysis should be enabled for the field depending
          * on the provided {@link Analyzer}
          */
+        @Override
         protected Query createFieldQuery(Analyzer analyzer, BooleanClause.Occur operator, String field,
                                          String queryText, boolean quoted, int phraseSlop) {
             assert operator == BooleanClause.Occur.SHOULD || operator == BooleanClause.Occur.MUST;
diff --git a/server/src/main/java/org/elasticsearch/index/search/MultiMatchQuery.java b/server/src/main/java/org/elasticsearch/index/search/MultiMatchQuery.java
index 017317279e5..6f57faba001 100644
--- a/server/src/main/java/org/elasticsearch/index/search/MultiMatchQuery.java
+++ b/server/src/main/java/org/elasticsearch/index/search/MultiMatchQuery.java
@@ -120,7 +120,10 @@ public class MultiMatchQuery extends MatchQuery {
             return queries;
         }
 
-        public Query parseGroup(Type type, String field, Float boostValue, Object value, String minimumShouldMatch) throws IOException {
+        Query parseGroup(Type type, String field, Float boostValue, Object value, String minimumShouldMatch) throws IOException {
+            if (context.fieldMapper(field) == null) {
+                return null;  // indicates to the caller that this field is unmapped and should be disregarded
+            }
             return parseAndApply(type, field, value, minimumShouldMatch, boostValue);
         }
 
diff --git a/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java
index 2d76b524ce6..969a59131c0 100644
--- a/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java
+++ b/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java
@@ -687,6 +687,26 @@ public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase<SimpleQ
         assertEquals(expected, query);
     }
 
+    /**
+     * Test for behavior reported in https://github.com/elastic/elasticsearch/issues/34708
+     * Unmapped field can lead to MatchNoDocsQuerys in disjunction queries. If tokens are eliminated (e.g. because
+     * the tokenizer removed them as punctuation) on regular fields, this can leave only MatchNoDocsQuerys in the
+     * disjunction clause. Instead those disjunctions should be eliminated completely.
+     */
+    public void testUnmappedFieldNoTokenWithAndOperator() throws IOException {
+        Query query = new SimpleQueryStringBuilder("first & second")
+                .field(STRING_FIELD_NAME)
+                .field("unmapped")
+                .field("another_unmapped")
+                .defaultOperator(Operator.AND)
+                .toQuery(createShardContext());
+        BooleanQuery expected = new BooleanQuery.Builder()
+                .add(new TermQuery(new Term(STRING_FIELD_NAME, "first")), BooleanClause.Occur.MUST)
+                .add(new TermQuery(new Term(STRING_FIELD_NAME, "second")), BooleanClause.Occur.MUST)
+                .build();
+        assertEquals(expected, query);
+    }
+
     private static IndexMetaData newIndexMeta(String name, Settings oldIndexSettings, Settings indexSettings) {
         Settings build = Settings.builder().put(oldIndexSettings)
             .put(indexSettings)