Improve exists query rewrite. (#97159)

The ExistsQueryBuilder rewrite only depends on information that exists in mappings. Therefor the rewrite can be done as index metadata rewrite. Practically this means the rewrite can occur during can match phase before opening a searcher or doing any potential refresh.

The `getMatchingFieldNames(...)` method was moved from SearchExecutionContext to QueryRewriteContext.

As an additional change, the QueryRewriteContext#doSearchRewrite(...) method now by default delegates to doIndexMetadataRewrite(...). This is because a search rewrite by default should be compatible with a index metadata rewrite.

Author: martijnvg

docs/changelog/97159.yaml

@@ -0,0 +1,5 @@
+pr: 97159
+summary: Improve exists query rewrite
+area: Search
+type: enhancement
+issues: []

server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java

@@ -322,7 +322,7 @@ protected QueryBuilder doCoordinatorRewrite(final CoordinatorRewriteContext coor
      * @return A {@link QueryBuilder} representing the rewritten query.
      */
     protected QueryBuilder doSearchRewrite(final SearchExecutionContext searchExecutionContext) throws IOException {
-        return this;
+        return doIndexMetadataRewrite(searchExecutionContext);
     }
 
     /**

server/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java

@@ -66,14 +66,12 @@ public String fieldName() {
     }
 
     @Override
-    protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException {
-        SearchExecutionContext context = queryRewriteContext.convertToSearchExecutionContext();
-        if (context != null) {
-            if (getMappedFields(context, fieldName).isEmpty()) {
-                return new MatchNoneQueryBuilder();
-            }
+    protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
+        if (getMappedFields(context, fieldName).isEmpty()) {
+            return new MatchNoneQueryBuilder();
+        } else {
+            return this;
         }
-        return super.doRewrite(queryRewriteContext);
     }
 
     @Override
@@ -153,7 +151,7 @@ public static Query newFilter(SearchExecutionContext context, String fieldPatter
         return new ConstantScoreQuery(boolFilterBuilder.build());
     }
 
-    private static Collection<String> getMappedFields(SearchExecutionContext context, String fieldPattern) {
+    private static Collection<String> getMappedFields(QueryRewriteContext context, String fieldPattern) {
         Set<String> matchingFieldNames = context.getMatchingFieldNames(fieldPattern);
         if (matchingFieldNames.isEmpty()) {
             // might be an object field, so try matching it as an object prefix pattern

server/src/main/java/org/elasticsearch/index/query/MatchPhraseQueryBuilder.java

@@ -156,11 +156,6 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
         builder.endObject();
     }
 
-    @Override
-    protected QueryBuilder doSearchRewrite(SearchExecutionContext searchExecutionContext) throws IOException {
-        return doIndexMetadataRewrite(searchExecutionContext);
-    }
-
     @Override
     protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
         // If we're using the default keyword analyzer then we can rewrite this to a TermQueryBuilder

server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java

@@ -10,6 +10,7 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.util.concurrent.CountDown;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexSettings;
@@ -26,9 +27,11 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.function.BooleanSupplier;
 import java.util.function.LongSupplier;
@@ -289,4 +292,34 @@ public boolean indexMatches(String pattern) {
         assert indexNameMatcher != null;
         return indexNameMatcher.test(pattern);
     }
+
+    /**
+     * Returns the names of all mapped fields that match a given pattern
+     *
+     * All names returned by this method are guaranteed to resolve to a
+     * MappedFieldType if passed to {@link #getFieldType(String)}
+     *
+     * @param pattern the field name pattern
+     */
+    public Set<String> getMatchingFieldNames(String pattern) {
+        if (runtimeMappings.isEmpty()) {
+            return mappingLookup.getMatchingFieldNames(pattern);
+        }
+        Set<String> matches = new HashSet<>(mappingLookup.getMatchingFieldNames(pattern));
+        if ("*".equals(pattern)) {
+            matches.addAll(runtimeMappings.keySet());
+        } else if (Regex.isSimpleMatchPattern(pattern) == false) {
+            // no wildcard
+            if (runtimeMappings.containsKey(pattern)) {
+                matches.add(pattern);
+            }
+        } else {
+            for (String name : runtimeMappings.keySet()) {
+                if (Regex.simpleMatch(pattern, name)) {
+                    matches.add(name);
+                }
+            }
+        }
+        return matches;
+    }
 }

server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java

@@ -24,7 +24,6 @@
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.lucene.search.Queries;
-import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.ClusterSettings;
 import org.elasticsearch.core.CheckedFunction;
 import org.elasticsearch.index.Index;
@@ -311,36 +310,6 @@ public boolean hasMappings() {
         return mappingLookup.hasMappings();
     }
 
-    /**
-     * Returns the names of all mapped fields that match a given pattern
-     *
-     * All names returned by this method are guaranteed to resolve to a
-     * MappedFieldType if passed to {@link #getFieldType(String)}
-     *
-     * @param pattern the field name pattern
-     */
-    public Set<String> getMatchingFieldNames(String pattern) {
-        if (runtimeMappings.isEmpty()) {
-            return mappingLookup.getMatchingFieldNames(pattern);
-        }
-        Set<String> matches = new HashSet<>(mappingLookup.getMatchingFieldNames(pattern));
-        if ("*".equals(pattern)) {
-            matches.addAll(runtimeMappings.keySet());
-        } else if (Regex.isSimpleMatchPattern(pattern) == false) {
-            // no wildcard
-            if (runtimeMappings.containsKey(pattern)) {
-                matches.add(pattern);
-            }
-        } else {
-            for (String name : runtimeMappings.keySet()) {
-                if (Regex.simpleMatch(pattern, name)) {
-                    matches.add(name);
-                }
-            }
-        }
-        return matches;
-    }
-
     /**
      * Returns true if the field identified by the provided name is mapped, false otherwise
      */

server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java

@@ -167,11 +167,6 @@ protected void addExtraXContent(XContentBuilder builder, Params params) throws I
         }
     }
 
-    @Override
-    protected QueryBuilder doSearchRewrite(SearchExecutionContext searchExecutionContext) throws IOException {
-        return doIndexMetadataRewrite(searchExecutionContext);
-    }
-
     @Override
     protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
         MappedFieldType fieldType = context.getFieldType(this.fieldName);

server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java

@@ -24,6 +24,7 @@
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.sameInstance;
 
 public class ExistsQueryBuilderTests extends AbstractQueryTestCase<ExistsQueryBuilder> {
     @Override
@@ -121,4 +122,20 @@ public void testFromJson() throws IOException {
         assertEquals(json, 42.0, parsed.boost(), 0.0001);
         assertEquals(json, "user", parsed.fieldName());
     }
+
+    public void testRewriteIndexQueryToMatchNone() throws IOException {
+        ExistsQueryBuilder query = QueryBuilders.existsQuery("does_not_exist");
+        for (QueryRewriteContext context : new QueryRewriteContext[] { createSearchExecutionContext(), createQueryRewriteContext() }) {
+            QueryBuilder rewritten = query.rewrite(context);
+            assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class));
+        }
+    }
+
+    public void testRewriteIndexQueryToNotMatchNone() throws IOException {
+        ExistsQueryBuilder query = QueryBuilders.existsQuery(KEYWORD_FIELD_NAME);
+        for (QueryRewriteContext context : new QueryRewriteContext[] { createSearchExecutionContext(), createQueryRewriteContext() }) {
+            QueryBuilder rewritten = query.rewrite(context);
+            assertThat(rewritten, sameInstance(query));
+        }
+    }
 }