Rewrite exists queries to a match_all query when all documents ma…

…tch. Since `BooleanQuery` has special rewrite rules in case some clauses are instances of `MatchAllDocsQuery` this can help simplify queries. I think this change will be especially helpful when elastic#22640 is implemented.
jpountz · Aug 16, 2017 · 2932dc1 · 2932dc1
1 parent 2978b5d
commit 2932dc1
Show file tree

Hide file tree

Showing 6 changed files with 79 additions and 10 deletions.
diff --git a/core/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java
@@ -22,7 +22,12 @@
 import org.apache.lucene.document.Field;
 import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.index.IndexableField;
+import org.apache.lucene.index.Term;
+import org.apache.lucene.index.TermContext;
+import org.apache.lucene.search.ConstantScoreQuery;
+import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TermQuery;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -183,7 +188,24 @@ public Query termQuery(Object value, QueryShardContext context) {
             if (isEnabled() == false) {
                 throw new IllegalStateException("Cannot run [exists] queries if the [_field_names] field is disabled");
             }
-            return super.termQuery(value, context);
+            failIfNotIndexed(); // should never fail since we do not allow this field to be not indexed
+            Term term = new Term(name(), indexedValueForSearch(value));
+            TermContext termContext;
+            try {
+                termContext = TermContext.build(context.getIndexReader().getContext(), term);
+            } catch (IOException e) {
+                throw new IllegalStateException(e);
+            }
+            if (termContext.docFreq() == context.getIndexReader().maxDoc()) {
+                // what should be the common case: all documents match
+                // BooleanQuery has special rewrite rules for MatchAllDocsQuery so this
+                // can help simplify the query
+                return new MatchAllDocsQuery();
+            } else {
+                // Make sure to pass the term context to the term query so that we do not
+                // pay the price for looking up the term in the terms dict twice
+                return new ConstantScoreQuery(new TermQuery(term, termContext));
+            }
         }
     }
 

diff --git a/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java
@@ -143,8 +143,7 @@ public static Query newFilter(QueryShardContext context, String fieldPattern) {
         }
 
         if (fields.size() == 1) {
-            Query filter = fieldNamesFieldType.termQuery(fields.iterator().next(), context);
-            return new ConstantScoreQuery(filter);
+            return fieldNamesFieldType.termQuery(fields.iterator().next(), context);
         }
 
         BooleanQuery.Builder boolFilterBuilder = new BooleanQuery.Builder();

diff --git a/core/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldTypeTests.java
@@ -18,13 +18,29 @@
  */
 package org.elasticsearch.index.mapper;
 
+import org.apache.lucene.document.Field.Store;
+import org.apache.lucene.document.StringField;
+import org.apache.lucene.index.DirectoryReader;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.Term;
+import org.apache.lucene.search.ConstantScoreQuery;
+import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.util.IOUtils;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.mapper.FieldNamesFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
+import org.elasticsearch.index.mapper.ParseContext.Document;
+import org.elasticsearch.index.query.QueryShardContext;
+import org.elasticsearch.test.IndexSettingsModule;
 import org.junit.Before;
 
+import java.io.IOException;
+
 public class FieldNamesFieldTypeTests extends FieldTypeTestCase {
     @Override
     protected MappedFieldType createDefaultFieldType() {
@@ -42,14 +58,39 @@ public void modify(MappedFieldType ft) {
         });
     }
 
-    public void testTermQuery() {
+    public void testTermQuery() throws IOException {
+        Directory dir = newDirectory();
+        IndexWriter w = new IndexWriter(dir, newIndexWriterConfig());
+        Document doc = new Document();
+        StringField fieldNames = new StringField(FieldNamesFieldMapper.CONTENT_TYPE, "field_name", Store.NO);
+        doc.add(fieldNames);
+        w.addDocument(doc);
+        IndexReader reader = DirectoryReader.open(w);
+        IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(randomAlphaOfLengthBetween(1, 10), Settings.EMPTY);
+        QueryShardContext context = new QueryShardContext(0, idxSettings, null, null, null, null, null, null, null, reader, null);
+
         FieldNamesFieldMapper.FieldNamesFieldType type = new FieldNamesFieldMapper.FieldNamesFieldType();
         type.setName(FieldNamesFieldMapper.CONTENT_TYPE);
         type.setEnabled(true);
-        Query termQuery = type.termQuery("field_name", null);
-        assertEquals(new TermQuery(new Term(FieldNamesFieldMapper.CONTENT_TYPE, "field_name")), termQuery);
+
+        Query termQuery = type.termQuery("field_name", context);
+        // all docs have the field
+        assertEquals(new MatchAllDocsQuery(), termQuery);
+
+        fieldNames.setStringValue("random_field_name");
+        w.addDocument(doc);
+        reader.close();
+        reader = DirectoryReader.open(w);
+        context = new QueryShardContext(0, idxSettings, null, null, null, null, null, null, null, reader, null);
+
+        termQuery = type.termQuery("field_name", context);
+        // not all docs have the field
+        assertEquals(new ConstantScoreQuery(new TermQuery(new Term(FieldNamesFieldMapper.CONTENT_TYPE, "field_name"))), termQuery);
+
         type.setEnabled(false);
         IllegalStateException e = expectThrows(IllegalStateException.class, () -> type.termQuery("field_name", null));
         assertEquals("Cannot run [exists] queries if the [_field_names] field is disabled", e.getMessage());
+
+        IOUtils.close(reader, w, dir);
     }
 }
diff --git a/core/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java
@@ -22,9 +22,9 @@
 import org.apache.lucene.search.BooleanClause;
 import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.ConstantScoreQuery;
+import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.MatchNoDocsQuery;
 import org.apache.lucene.search.Query;
-import org.apache.lucene.search.TermQuery;
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.test.AbstractQueryTestCase;
@@ -67,9 +67,7 @@ protected void doAssertLuceneQuery(ExistsQueryBuilder queryBuilder, Query query,
         } else if (fields.size() == 1) {
             assertThat(query, instanceOf(ConstantScoreQuery.class));
             ConstantScoreQuery constantScoreQuery = (ConstantScoreQuery) query;
-            assertThat(constantScoreQuery.getQuery(), instanceOf(TermQuery.class));
-            TermQuery termQuery = (TermQuery) constantScoreQuery.getQuery();
-            assertEquals(fields.iterator().next(), termQuery.getTerm().text());
+            assertThat(constantScoreQuery.getQuery(), instanceOf(MatchAllDocsQuery.class));
         } else {
             assertThat(query, instanceOf(ConstantScoreQuery.class));
             ConstantScoreQuery constantScoreQuery = (ConstantScoreQuery) query;

diff --git a/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java
@@ -54,6 +54,7 @@
 import org.elasticsearch.common.unit.Fuzziness;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.index.mapper.FieldNamesFieldMapper;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.search.QueryStringQueryParser;
 import org.elasticsearch.search.internal.SearchContext;

diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java
@@ -21,6 +21,8 @@
 
 import com.fasterxml.jackson.core.io.JsonStringEncoder;
 
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.MultiReader;
 import org.apache.lucene.search.BoostQuery;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.TermQuery;
@@ -1100,6 +1102,12 @@ public void close() throws IOException {
         }
 
         QueryShardContext createShardContext() {
+            IndexReader reader;
+            try {
+                reader = new MultiReader();
+            } catch (IOException e) {
+                throw new AssertionError(e);
+            }
             return new QueryShardContext(0, idxSettings, bitsetFilterCache, indexFieldDataService::getForField, mapperService,
                 similarityService, scriptService, xContentRegistry, namedWriteableRegistry, this.client, null, () -> nowInMillis, null);
         }