diff --git a/server/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java b/server/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java index e8e858b483521..6f733c9f0a5f1 100644 --- a/server/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java +++ b/server/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java @@ -340,9 +340,8 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio @Override public boolean isCacheable(LeafReaderContext ctx) { - // If minScore is not null, then matches depend on statistics of the - // top-level reader. - return minScore == null; + // the sub-query/filters should be cached independently when the score is not needed. + return false; } } diff --git a/server/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreQuery.java b/server/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreQuery.java index c33f588ac3670..cfbe9440998e3 100644 --- a/server/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreQuery.java +++ b/server/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreQuery.java @@ -151,8 +151,8 @@ private ScoreScript makeScoreScript(LeafReaderContext context) throws IOExceptio @Override public boolean isCacheable(LeafReaderContext ctx) { - // If minScore is not null, then matches depend on statistics of the top-level reader. - return minScore == null; + // the sub-query should be cached independently when the score is not needed + return false; } }; } diff --git a/server/src/test/java/org/elasticsearch/index/query/ScriptScoreQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/ScriptScoreQueryBuilderTests.java index 72d18fad32c84..dc84705168f3f 100644 --- a/server/src/test/java/org/elasticsearch/index/query/ScriptScoreQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/ScriptScoreQueryBuilderTests.java @@ -8,8 +8,15 @@ package org.elasticsearch.index.query; +import org.apache.lucene.document.Document; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.RandomIndexWriter; +import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; +import org.apache.lucene.search.ScoreMode; +import org.apache.lucene.search.Weight; +import org.apache.lucene.store.Directory; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.lucene.search.function.ScriptScoreQuery; import org.elasticsearch.index.query.functionscore.ScriptScoreQueryBuilder; @@ -23,6 +30,7 @@ import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.Matchers.greaterThan; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -88,14 +96,33 @@ public void testIllegalArguments() { */ @Override public void testCacheability() throws IOException { + Directory directory = newDirectory(); + RandomIndexWriter iw = new RandomIndexWriter(random(), directory); + iw.addDocument(new Document()); + final IndexSearcher searcher = new IndexSearcher(iw.getReader()); + iw.close(); + assertThat(searcher.getIndexReader().leaves().size(), greaterThan(0)); + Script script = new Script(ScriptType.INLINE, MockScriptEngine.NAME, "1", Collections.emptyMap()); ScriptScoreQueryBuilder queryBuilder = new ScriptScoreQueryBuilder( new TermQueryBuilder(KEYWORD_FIELD_NAME, "value"), script); - SearchExecutionContext context = createSearchExecutionContext(); + SearchExecutionContext context = createSearchExecutionContext(searcher); QueryBuilder rewriteQuery = rewriteQuery(queryBuilder, new SearchExecutionContext(context)); - assertNotNull(rewriteQuery.toQuery(context)); + Query luceneQuery = rewriteQuery.toQuery(context); + assertNotNull(luceneQuery); assertTrue("query should be cacheable: " + queryBuilder.toString(), context.isCacheable()); + + // test query cache + if (rewriteQuery instanceof MatchNoneQueryBuilder == false) { + Weight queryWeight = context.searcher().createWeight(searcher.rewrite(luceneQuery), ScoreMode.COMPLETE, 1.0f); + for (LeafReaderContext ctx : context.getIndexReader().leaves()) { + assertFalse("" + searcher.rewrite(luceneQuery) + " " + rewriteQuery.toString(), queryWeight.isCacheable(ctx)); + } + } + + searcher.getIndexReader().close(); + directory.close(); } @Override diff --git a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java index fc1b76b998d49..4788bb0d49694 100644 --- a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java @@ -10,11 +10,18 @@ import com.fasterxml.jackson.core.JsonParseException; +import org.apache.lucene.document.Document; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.RandomIndexWriter; import org.apache.lucene.index.Term; +import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; +import org.apache.lucene.search.ScoreMode; import org.apache.lucene.search.TermQuery; +import org.apache.lucene.search.Weight; +import org.apache.lucene.store.Directory; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; @@ -70,6 +77,7 @@ import static org.hamcrest.Matchers.closeTo; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.nullValue; @@ -815,34 +823,72 @@ public List> getScoreFunctions() { */ @Override public void testCacheability() throws IOException { + Directory directory = newDirectory(); + RandomIndexWriter iw = new RandomIndexWriter(random(), directory); + iw.addDocument(new Document()); + final IndexSearcher searcher = new IndexSearcher(iw.getReader()); + iw.close(); + assertThat(searcher.getIndexReader().leaves().size(), greaterThan(0)); + FunctionScoreQueryBuilder queryBuilder = createTestQueryBuilder(); - boolean isCacheable = isCacheable(queryBuilder); - SearchExecutionContext context = createSearchExecutionContext(); + boolean requestCache = isCacheable(queryBuilder); + SearchExecutionContext context = createSearchExecutionContext(searcher); QueryBuilder rewriteQuery = rewriteQuery(queryBuilder, new SearchExecutionContext(context)); assertNotNull(rewriteQuery.toQuery(context)); - // we occasionally need to update the expected "isCacheable" flag after rewrite for MatchNoneQueryBuilder + // we occasionally need to update the expected request cache flag after rewrite to MatchNoneQueryBuilder if (rewriteQuery instanceof MatchNoneQueryBuilder) { - isCacheable = true; + requestCache = true; + } + assertEquals("query should " + (requestCache ? "" : "not") + " be eligible for the request cache: " + queryBuilder.toString(), + requestCache, context.isCacheable()); + + // test query cache + if (rewriteQuery instanceof MatchNoneQueryBuilder == false) { + Query luceneQuery = rewriteQuery.toQuery(context); + Weight queryWeight = context.searcher().createWeight(searcher.rewrite(luceneQuery), ScoreMode.COMPLETE, 1.0f); + for (LeafReaderContext ctx : context.getIndexReader().leaves()) { + assertFalse(queryWeight.isCacheable(ctx)); + } } - assertEquals("query should " + (isCacheable ? "" : "not") + " be cacheable: " + queryBuilder.toString(), isCacheable, - context.isCacheable()); ScoreFunctionBuilder scriptScoreFunction = new ScriptScoreFunctionBuilder( new Script(ScriptType.INLINE, MockScriptEngine.NAME, "1", Collections.emptyMap())); queryBuilder = new FunctionScoreQueryBuilder(new FilterFunctionBuilder[] { new FilterFunctionBuilder(RandomQueryBuilder.createQuery(random()), scriptScoreFunction) }); - context = createSearchExecutionContext(); + context = createSearchExecutionContext(searcher); rewriteQuery = rewriteQuery(queryBuilder, new SearchExecutionContext(context)); assertNotNull(rewriteQuery.toQuery(context)); - assertTrue("function script query should be cacheable" + queryBuilder.toString(), context.isCacheable()); + assertTrue("function script query should be eligible for the request cache: " + queryBuilder.toString(), + context.isCacheable()); + + // test query cache + if (rewriteQuery instanceof MatchNoneQueryBuilder == false) { + Query luceneQuery = rewriteQuery.toQuery(context); + Weight queryWeight = context.searcher().createWeight(searcher.rewrite(luceneQuery), ScoreMode.COMPLETE, 1.0f); + for (LeafReaderContext ctx : context.getIndexReader().leaves()) { + assertFalse(queryWeight.isCacheable(ctx)); + } + } RandomScoreFunctionBuilder randomScoreFunctionBuilder = new RandomScoreFunctionBuilderWithFixedSeed(); queryBuilder = new FunctionScoreQueryBuilder(new FilterFunctionBuilder[] { new FilterFunctionBuilder(RandomQueryBuilder.createQuery(random()), randomScoreFunctionBuilder) }); - context = createSearchExecutionContext(); + context = createSearchExecutionContext(searcher); rewriteQuery = rewriteQuery(queryBuilder, new SearchExecutionContext(context)); assertNotNull(rewriteQuery.toQuery(context)); - assertFalse("function random query should not be cacheable: " + queryBuilder.toString(), context.isCacheable()); + assertFalse("function random query should not be eligible for the request cache: " + queryBuilder.toString(), + context.isCacheable()); + + // test query cache + if (rewriteQuery instanceof MatchNoneQueryBuilder == false) { + Query luceneQuery = rewriteQuery.toQuery(context); + Weight queryWeight = context.searcher().createWeight(searcher.rewrite(luceneQuery), ScoreMode.COMPLETE, 1.0f); + for (LeafReaderContext ctx : context.getIndexReader().leaves()) { + assertFalse(queryWeight.isCacheable(ctx)); + } + } + searcher.getIndexReader().close(); + directory.close(); } private boolean isCacheable(FunctionScoreQueryBuilder queryBuilder) {