Require arguments for QueryShardContext creation. (#21196)

The `IndexService#newQueryShardContext()` method creates a QueryShardContext on
shard `0`, with a `null` reader and that uses `System.currentTimeMillis()` to
resolve `now`. This may hide bugs, since the shard id is sometimes used for
query parsing (it is used to salt random score generation in `function_score`),
passing a `null` reader disables query rewriting and for some use-cases, it is
simply not ok to rely on the current timestamp (eg. percolation). So this pull
request removes this method and instead requires that all call sites provide
these parameters explicitly.

10 files changed, 59 insertions, 33 deletions

diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java
index e7c032a120..f9034f6a29 100644
--- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java
+++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java
@@ -352,7 +352,9 @@ public class MetaDataCreateIndexService extends AbstractComponent {
                                 throw mpe;
                             }
 
-                            final QueryShardContext queryShardContext = indexService.newQueryShardContext();
+                            // the context is only used for validation so it's fine to pass fake values for the shard id and the current
+                            // timestamp
+                            final QueryShardContext queryShardContext = indexService.newQueryShardContext(0, null, () -> 0L);
                             for (Alias alias : request.aliases()) {
                                 if (Strings.hasLength(alias.filter())) {
                                     aliasValidator.validateAliasFilter(alias.name(), alias.filter(), queryShardContext);
diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexAliasesService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexAliasesService.java
index 5b0c5a8406..b97b77de64 100644
--- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexAliasesService.java
+++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataIndexAliasesService.java
@@ -149,7 +149,9 @@ public class MetaDataIndexAliasesService extends AbstractComponent {
                             }
                             indices.put(action.getIndex(), indexService);
                         }
-                        aliasValidator.validateAliasFilter(alias, filter, indexService.newQueryShardContext());
+                        // the context is only used for validation so it's fine to pass fake values for the shard id and the current
+                        // timestamp
+                        aliasValidator.validateAliasFilter(alias, filter, indexService.newQueryShardContext(0, null, () -> 0L));
                     }
                 };
                 changed |= action.apply(newAliasValidator, metadata, index);
diff --git a/core/src/main/java/org/elasticsearch/index/IndexService.java b/core/src/main/java/org/elasticsearch/index/IndexService.java
index 0114d68b4e..5fc0d1d27c 100644
--- a/core/src/main/java/org/elasticsearch/index/IndexService.java
+++ b/core/src/main/java/org/elasticsearch/index/IndexService.java
@@ -145,7 +145,10 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust
         this.indexAnalyzers = registry.build(indexSettings);
         this.similarityService = similarityService;
         this.mapperService = new MapperService(indexSettings, indexAnalyzers, similarityService, mapperRegistry,
-            IndexService.this::newQueryShardContext);
+            // we parse all percolator queries as they would be parsed on shard 0
+            () -> newQueryShardContext(0, null, () -> {
+                throw new IllegalArgumentException("Percolator queries are not allowed to use the curent timestamp");
+            }));
         this.indexFieldData = new IndexFieldDataService(indexSettings, indicesFieldDataCache, circuitBreakerService, mapperService);
         this.shardStoreDeleter = shardStoreDeleter;
         this.bigArrays = bigArrays;
@@ -453,7 +456,10 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust
 
     /**
      * Creates a new QueryShardContext. The context has not types set yet, if types are required set them via
-     * {@link QueryShardContext#setTypes(String...)}
+     * {@link QueryShardContext#setTypes(String...)}.
+     *
+     * Passing a {@code null} {@link IndexReader} will return a valid context, however it won't be able to make
+     * {@link IndexReader}-specific optimizations, such as rewriting containing range queries.
      */
     public QueryShardContext newQueryShardContext(int shardId, IndexReader indexReader, LongSupplier nowInMillis) {
         return new QueryShardContext(
@@ -465,15 +471,6 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust
     }
 
     /**
-     * Creates a new QueryShardContext. The context has not types set yet, if types are required set them via
-     * {@link QueryShardContext#setTypes(String...)}. This context may be used for query parsing but cannot be
-     * used for rewriting since it does not know about the current {@link IndexReader}.
-     */
-    public QueryShardContext newQueryShardContext() {
-        return newQueryShardContext(0, null, System::currentTimeMillis);
-    }
-
-    /**
      * The {@link ThreadPool} to use for this index.
      */
     public ThreadPool getThreadPool() {
diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java
index ccca0af652..c8b5cf174f 100644
--- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java
+++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java
@@ -86,7 +86,9 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier {
         return mapperService;
     }
 
-    /** Return the current {@link IndexReader}, or {@code null} if we are on the coordinating node. */
+    /** Return the current {@link IndexReader}, or {@code null} if no index reader is available, for
+     *  instance if we are on the coordinating node or if this rewrite context is used to index
+     *  queries (percolation). */
     public IndexReader getIndexReader() {
         return reader;
     }
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/CustomBoostMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/CustomBoostMappingTests.java
index 9bc87e874f..391f987e71 100644
--- a/core/src/test/java/org/elasticsearch/index/mapper/CustomBoostMappingTests.java
+++ b/core/src/test/java/org/elasticsearch/index/mapper/CustomBoostMappingTests.java
@@ -97,7 +97,7 @@ public class CustomBoostMappingTests extends ESSingleNodeTestCase {
                     .startObject("date_field").field("type", "date").field("boost", 9.0f).endObject()
                     .endObject().endObject().endObject().string();
             IndexService indexService = createIndex("test", BW_SETTINGS);
-            QueryShardContext context = indexService.newQueryShardContext();
+            QueryShardContext context = indexService.newQueryShardContext(0, null, () -> 0L);
             DocumentMapper mapper = indexService.mapperService().documentMapperParser().parse("type", new CompressedXContent(mapping));
             DocumentFieldMappers fieldMappers = mapper.mappers();
             assertThat(fieldMappers.getMapper("s_field").fieldType().termQuery("0", context), instanceOf(TermQuery.class));
@@ -150,7 +150,7 @@ public class CustomBoostMappingTests extends ESSingleNodeTestCase {
                     .startObject("date_field").field("type", "date").field("boost", 9.0f).endObject()
                     .endObject().endObject().endObject().string();
             IndexService indexService = createIndex("text");
-            QueryShardContext context = indexService.newQueryShardContext();
+            QueryShardContext context = indexService.newQueryShardContext(0, null, () -> 0L);
             DocumentMapper mapper = indexService.mapperService().documentMapperParser().parse("type", new CompressedXContent(mapping));
             DocumentFieldMappers fieldMappers = mapper.mappers();
             assertThat(fieldMappers.getMapper("s_field").fieldType().termQuery("0", context), instanceOf(BoostQuery.class));
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DoubleIndexingDocTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DoubleIndexingDocTests.java
index b434b41242..af217030f2 100644
--- a/core/src/test/java/org/elasticsearch/index/mapper/DoubleIndexingDocTests.java
+++ b/core/src/test/java/org/elasticsearch/index/mapper/DoubleIndexingDocTests.java
@@ -45,7 +45,7 @@ public class DoubleIndexingDocTests extends ESSingleNodeTestCase {
         IndexService index = createIndex("test");
         client().admin().indices().preparePutMapping("test").setType("type").setSource(mapping).get();
         DocumentMapper mapper = index.mapperService().documentMapper("type");
-        QueryShardContext context = index.newQueryShardContext();
+        QueryShardContext context = index.newQueryShardContext(0, null, () -> 0L);
 
         ParsedDocument doc = mapper.parse("test", "type", "1", XContentFactory.jsonBuilder()
                 .startObject()
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java
index 44c973748d..72f9d09808 100644
--- a/core/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java
+++ b/core/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java
@@ -29,6 +29,7 @@ import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.IndexService;
+import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.indices.mapper.MapperRegistry;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESSingleNodeTestCase;
@@ -39,6 +40,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.function.Supplier;
 
 import static org.hamcrest.Matchers.closeTo;
 import static org.hamcrest.Matchers.is;
@@ -59,8 +61,11 @@ public class ExternalFieldMapperTests extends ESSingleNodeTestCase {
                 Collections.singletonMap(ExternalMapperPlugin.EXTERNAL, new ExternalMapper.TypeParser(ExternalMapperPlugin.EXTERNAL, "foo")),
                 Collections.singletonMap(ExternalMetadataMapper.CONTENT_TYPE, new ExternalMetadataMapper.TypeParser()));
 
+        Supplier<QueryShardContext> queryShardContext = () -> {
+            return indexService.newQueryShardContext(0, null, () -> { throw new UnsupportedOperationException(); });
+        };
         DocumentMapperParser parser = new DocumentMapperParser(indexService.getIndexSettings(), indexService.mapperService(),
-                indexService.getIndexAnalyzers(), indexService.similarityService(), mapperRegistry, indexService::newQueryShardContext);
+                indexService.getIndexAnalyzers(), indexService.similarityService(), mapperRegistry, queryShardContext);
         DocumentMapper documentMapper = parser.parse("type", new CompressedXContent(
                 XContentFactory.jsonBuilder().startObject().startObject("type")
                 .startObject(ExternalMetadataMapper.CONTENT_TYPE)
@@ -108,8 +113,11 @@ public class ExternalFieldMapperTests extends ESSingleNodeTestCase {
         mapperParsers.put(KeywordFieldMapper.CONTENT_TYPE, new KeywordFieldMapper.TypeParser());
         MapperRegistry mapperRegistry = new MapperRegistry(mapperParsers, Collections.emptyMap());
 
+        Supplier<QueryShardContext> queryShardContext = () -> {
+            return indexService.newQueryShardContext(0, null, () -> { throw new UnsupportedOperationException(); });
+        };
         DocumentMapperParser parser = new DocumentMapperParser(indexService.getIndexSettings(), indexService.mapperService(),
-                indexService.getIndexAnalyzers(), indexService.similarityService(), mapperRegistry, indexService::newQueryShardContext);
+                indexService.getIndexAnalyzers(), indexService.similarityService(), mapperRegistry, queryShardContext);
 
         DocumentMapper documentMapper = parser.parse("type", new CompressedXContent(
                 XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties")
@@ -178,8 +186,11 @@ public class ExternalFieldMapperTests extends ESSingleNodeTestCase {
         mapperParsers.put(TextFieldMapper.CONTENT_TYPE, new TextFieldMapper.TypeParser());
         MapperRegistry mapperRegistry = new MapperRegistry(mapperParsers, Collections.emptyMap());
 
+        Supplier<QueryShardContext> queryShardContext = () -> {
+            return indexService.newQueryShardContext(0, null, () -> { throw new UnsupportedOperationException(); });
+        };
         DocumentMapperParser parser = new DocumentMapperParser(indexService.getIndexSettings(), indexService.mapperService(),
-                indexService.getIndexAnalyzers(), indexService.similarityService(), mapperRegistry, indexService::newQueryShardContext);
+                indexService.getIndexAnalyzers(), indexService.similarityService(), mapperRegistry, queryShardContext);
 
         DocumentMapper documentMapper = parser.parse("type", new CompressedXContent(
                 XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties")
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java
index 544764a9b5..5bc70350b1 100644
--- a/core/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java
+++ b/core/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java
@@ -27,6 +27,7 @@ import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.IndexService;
+import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.indices.IndicesModule;
 import org.elasticsearch.indices.mapper.MapperRegistry;
 import org.elasticsearch.test.ESSingleNodeTestCase;
@@ -38,6 +39,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.SortedSet;
 import java.util.TreeSet;
+import java.util.function.Supplier;
 
 public class FieldNamesFieldMapperTests extends ESSingleNodeTestCase {
 
@@ -231,9 +233,12 @@ public class FieldNamesFieldMapperTests extends ESSingleNodeTestCase {
             Collections.singletonMap("_dummy", new DummyMetadataFieldMapper.TypeParser())
         );
         final MapperRegistry mapperRegistry = indicesModule.getMapperRegistry();
-        MapperService mapperService = new MapperService(indexService.getIndexSettings(), indexService.getIndexAnalyzers(), indexService.similarityService(), mapperRegistry, indexService::newQueryShardContext);
+        Supplier<QueryShardContext> queryShardContext = () -> {
+            return indexService.newQueryShardContext(0, null, () -> { throw new UnsupportedOperationException(); });
+        };
+        MapperService mapperService = new MapperService(indexService.getIndexSettings(), indexService.getIndexAnalyzers(), indexService.similarityService(), mapperRegistry, queryShardContext);
         DocumentMapperParser parser = new DocumentMapperParser(indexService.getIndexSettings(), mapperService,
-                indexService.getIndexAnalyzers(), indexService.similarityService(), mapperRegistry, indexService::newQueryShardContext);
+                indexService.getIndexAnalyzers(), indexService.similarityService(), mapperRegistry, queryShardContext);
         String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject().string();
         DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping));
         ParsedDocument parsedDocument = mapper.parse("index", "type", "id", new BytesArray("{}"));
diff --git a/core/src/test/java/org/elasticsearch/index/query/plugin/CustomQueryParserIT.java b/core/src/test/java/org/elasticsearch/index/query/plugin/CustomQueryParserIT.java
index 67619a3bbe..c2c39be93b 100644
--- a/core/src/test/java/org/elasticsearch/index/query/plugin/CustomQueryParserIT.java
+++ b/core/src/test/java/org/elasticsearch/index/query/plugin/CustomQueryParserIT.java
@@ -74,7 +74,8 @@ public class CustomQueryParserIT extends ESIntegTestCase {
 
     private static QueryShardContext queryShardContext() {
         IndicesService indicesService = internalCluster().getDataNodeInstance(IndicesService.class);
-        return indicesService.indexServiceSafe(resolveIndex("index")).newQueryShardContext();
+        return indicesService.indexServiceSafe(resolveIndex("index")).newQueryShardContext(
+                randomInt(20), null, () -> { throw new UnsupportedOperationException(); });
     }
 
     //see #11120
diff --git a/core/src/test/java/org/elasticsearch/index/search/MultiMatchQueryTests.java b/core/src/test/java/org/elasticsearch/index/search/MultiMatchQueryTests.java
index 2454150be5..7d7b7a4cd6 100644
--- a/core/src/test/java/org/elasticsearch/index/search/MultiMatchQueryTests.java
+++ b/core/src/test/java/org/elasticsearch/index/search/MultiMatchQueryTests.java
@@ -78,7 +78,8 @@ public class MultiMatchQueryTests extends ESSingleNodeTestCase {
     }
 
     public void testCrossFieldMultiMatchQuery() throws IOException {
-        QueryShardContext queryShardContext = indexService.newQueryShardContext();
+        QueryShardContext queryShardContext = indexService.newQueryShardContext(
+                randomInt(20), null, () -> { throw new UnsupportedOperationException(); });
         queryShardContext.setAllowUnmappedFields(true);
         Query parsedQuery = multiMatchQuery("banon").field("name.first", 2).field("name.last", 3).field("foobar").type(MultiMatchQueryBuilder.Type.CROSS_FIELDS).toQuery(queryShardContext);
         try (Engine.Searcher searcher = indexService.getShard(0).acquireSearcher("test")) {
@@ -101,8 +102,9 @@ public class MultiMatchQueryTests extends ESSingleNodeTestCase {
         Term[] terms = new Term[] { new Term("foo", "baz"), new Term("bar", "baz") };
         float[] boosts = new float[] {2, 3};
         Query expected = BlendedTermQuery.booleanBlendedQuery(terms, boosts, false);
-        Query actual = MultiMatchQuery.blendTerm(indexService.newQueryShardContext(), new BytesRef("baz"), null, 1f,
-                new FieldAndFieldType(ft1, 2), new FieldAndFieldType(ft2, 3));
+        Query actual = MultiMatchQuery.blendTerm(
+                indexService.newQueryShardContext(randomInt(20), null, () -> { throw new UnsupportedOperationException(); }),
+                new BytesRef("baz"), null, 1f, new FieldAndFieldType(ft1, 2), new FieldAndFieldType(ft2, 3));
         assertEquals(expected, actual);
     }
 
@@ -116,8 +118,9 @@ public class MultiMatchQueryTests extends ESSingleNodeTestCase {
         Term[] terms = new Term[] { new Term("foo", "baz"), new Term("bar", "baz") };
         float[] boosts = new float[] {200, 30};
         Query expected = BlendedTermQuery.booleanBlendedQuery(terms, boosts, false);
-        Query actual = MultiMatchQuery.blendTerm(indexService.newQueryShardContext(), new BytesRef("baz"), null, 1f,
-                new FieldAndFieldType(ft1, 2), new FieldAndFieldType(ft2, 3));
+        Query actual = MultiMatchQuery.blendTerm(
+                indexService.newQueryShardContext(randomInt(20), null, () -> { throw new UnsupportedOperationException(); }),
+                new BytesRef("baz"), null, 1f, new FieldAndFieldType(ft1, 2), new FieldAndFieldType(ft2, 3));
         assertEquals(expected, actual);
     }
 
@@ -134,8 +137,9 @@ public class MultiMatchQueryTests extends ESSingleNodeTestCase {
         Term[] terms = new Term[] { new Term("foo", "baz") };
         float[] boosts = new float[] {2};
         Query expected = BlendedTermQuery.booleanBlendedQuery(terms, boosts, false);
-        Query actual = MultiMatchQuery.blendTerm(indexService.newQueryShardContext(), new BytesRef("baz"), null, 1f,
-                new FieldAndFieldType(ft1, 2), new FieldAndFieldType(ft2, 3));
+        Query actual = MultiMatchQuery.blendTerm(
+                indexService.newQueryShardContext(randomInt(20), null, () -> { throw new UnsupportedOperationException(); }),
+                new BytesRef("baz"), null, 1f, new FieldAndFieldType(ft1, 2), new FieldAndFieldType(ft2, 3));
         assertEquals(expected, actual);
     }
 
@@ -157,13 +161,15 @@ public class MultiMatchQueryTests extends ESSingleNodeTestCase {
                 .add(expectedClause1, Occur.SHOULD)
                 .add(expectedClause2, Occur.SHOULD)
                 .build();
-        Query actual = MultiMatchQuery.blendTerm(indexService.newQueryShardContext(), new BytesRef("baz"), null, 1f,
-                new FieldAndFieldType(ft1, 2), new FieldAndFieldType(ft2, 3));
+        Query actual = MultiMatchQuery.blendTerm(
+                indexService.newQueryShardContext(randomInt(20), null, () -> { throw new UnsupportedOperationException(); }),
+                new BytesRef("baz"), null, 1f, new FieldAndFieldType(ft1, 2), new FieldAndFieldType(ft2, 3));
         assertEquals(expected, actual);
     }
 
     public void testMultiMatchPrefixWithAllField() throws IOException {
-        QueryShardContext queryShardContext = indexService.newQueryShardContext();
+        QueryShardContext queryShardContext = indexService.newQueryShardContext(
+                randomInt(20), null, () -> { throw new UnsupportedOperationException(); });
         queryShardContext.setAllowUnmappedFields(true);
         Query parsedQuery =
             multiMatchQuery("foo").field("_all").type(MultiMatchQueryBuilder.Type.PHRASE_PREFIX).toQuery(queryShardContext);