diff options
author | Adrien Grand <jpountz@gmail.com> | 2016-11-02 09:48:49 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-11-02 09:48:49 +0100 |
commit | aa6cd93e0f860faf869e46386beea3836f635816 (patch) | |
tree | 933aff6cb175c172f7276523220cd5fdb83349c3 /core/src/main/java/org/elasticsearch | |
parent | 51717c882f23a5a5d3fae1a23477b29b4079ca4d (diff) |
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.
Diffstat (limited to 'core/src/main/java/org/elasticsearch')
4 files changed, 17 insertions, 14 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; } |