Skip to content

Commit 51b382f

Browse files
committed
Disable query cache for FunctionScoreQuery and ScriptScoreQuery (#74060)
This commit disables the query cache for the `FunctionScoreQuery` and the `ScriptScoreQuery`. These queries are not meant to be cached. If the score is not needed, we'll now cache the sub-query and filters independently since we don't want to keep an unused script in the cache. Closes #73925
1 parent e25a7e4 commit 51b382f

File tree

4 files changed

+89
-17
lines changed

4 files changed

+89
-17
lines changed

server/src/main/java/org/elasticsearch/common/lucene/search/function/FunctionScoreQuery.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -340,9 +340,8 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio
340340

341341
@Override
342342
public boolean isCacheable(LeafReaderContext ctx) {
343-
// If minScore is not null, then matches depend on statistics of the
344-
// top-level reader.
345-
return minScore == null;
343+
// the sub-query/filters should be cached independently when the score is not needed.
344+
return false;
346345
}
347346
}
348347

server/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreQuery.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ private ScoreScript makeScoreScript(LeafReaderContext context) throws IOExceptio
151151

152152
@Override
153153
public boolean isCacheable(LeafReaderContext ctx) {
154-
// If minScore is not null, then matches depend on statistics of the top-level reader.
155-
return minScore == null;
154+
// the sub-query should be cached independently when the score is not needed
155+
return false;
156156
}
157157
};
158158
}

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

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,15 @@
88

99
package org.elasticsearch.index.query;
1010

11+
import org.apache.lucene.document.Document;
12+
import org.apache.lucene.index.LeafReaderContext;
13+
import org.apache.lucene.index.RandomIndexWriter;
14+
import org.apache.lucene.search.IndexSearcher;
1115
import org.apache.lucene.search.MatchNoDocsQuery;
1216
import org.apache.lucene.search.Query;
17+
import org.apache.lucene.search.ScoreMode;
18+
import org.apache.lucene.search.Weight;
19+
import org.apache.lucene.store.Directory;
1320
import org.elasticsearch.ElasticsearchException;
1421
import org.elasticsearch.common.lucene.search.function.ScriptScoreQuery;
1522
import org.elasticsearch.index.query.functionscore.ScriptScoreQueryBuilder;
@@ -23,6 +30,7 @@
2330

2431
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
2532
import static org.hamcrest.CoreMatchers.instanceOf;
33+
import static org.hamcrest.Matchers.greaterThan;
2634
import static org.mockito.Mockito.mock;
2735
import static org.mockito.Mockito.when;
2836

@@ -88,14 +96,33 @@ public void testIllegalArguments() {
8896
*/
8997
@Override
9098
public void testCacheability() throws IOException {
99+
Directory directory = newDirectory();
100+
RandomIndexWriter iw = new RandomIndexWriter(random(), directory);
101+
iw.addDocument(new Document());
102+
final IndexSearcher searcher = new IndexSearcher(iw.getReader());
103+
iw.close();
104+
assertThat(searcher.getIndexReader().leaves().size(), greaterThan(0));
105+
91106
Script script = new Script(ScriptType.INLINE, MockScriptEngine.NAME, "1", Collections.emptyMap());
92107
ScriptScoreQueryBuilder queryBuilder = new ScriptScoreQueryBuilder(
93108
new TermQueryBuilder(KEYWORD_FIELD_NAME, "value"), script);
94109

95-
SearchExecutionContext context = createSearchExecutionContext();
110+
SearchExecutionContext context = createSearchExecutionContext(searcher);
96111
QueryBuilder rewriteQuery = rewriteQuery(queryBuilder, new SearchExecutionContext(context));
97-
assertNotNull(rewriteQuery.toQuery(context));
112+
Query luceneQuery = rewriteQuery.toQuery(context);
113+
assertNotNull(luceneQuery);
98114
assertTrue("query should be cacheable: " + queryBuilder.toString(), context.isCacheable());
115+
116+
// test query cache
117+
if (rewriteQuery instanceof MatchNoneQueryBuilder == false) {
118+
Weight queryWeight = context.searcher().createWeight(searcher.rewrite(luceneQuery), ScoreMode.COMPLETE, 1.0f);
119+
for (LeafReaderContext ctx : context.getIndexReader().leaves()) {
120+
assertFalse("" + searcher.rewrite(luceneQuery) + " " + rewriteQuery.toString(), queryWeight.isCacheable(ctx));
121+
}
122+
}
123+
124+
searcher.getIndexReader().close();
125+
directory.close();
99126
}
100127

101128
@Override

server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,18 @@
1010

1111
import com.fasterxml.jackson.core.JsonParseException;
1212

13+
import org.apache.lucene.document.Document;
14+
import org.apache.lucene.index.LeafReaderContext;
15+
import org.apache.lucene.index.RandomIndexWriter;
1316
import org.apache.lucene.index.Term;
17+
import org.apache.lucene.search.IndexSearcher;
1418
import org.apache.lucene.search.MatchAllDocsQuery;
1519
import org.apache.lucene.search.MatchNoDocsQuery;
1620
import org.apache.lucene.search.Query;
21+
import org.apache.lucene.search.ScoreMode;
1722
import org.apache.lucene.search.TermQuery;
23+
import org.apache.lucene.search.Weight;
24+
import org.apache.lucene.store.Directory;
1825
import org.elasticsearch.common.ParsingException;
1926
import org.elasticsearch.common.Strings;
2027
import org.elasticsearch.common.bytes.BytesReference;
@@ -70,6 +77,7 @@
7077
import static org.hamcrest.Matchers.closeTo;
7178
import static org.hamcrest.Matchers.containsString;
7279
import static org.hamcrest.Matchers.equalTo;
80+
import static org.hamcrest.Matchers.greaterThan;
7381
import static org.hamcrest.Matchers.instanceOf;
7482
import static org.hamcrest.Matchers.nullValue;
7583

@@ -815,34 +823,72 @@ public List<ScoreFunctionSpec<?>> getScoreFunctions() {
815823
*/
816824
@Override
817825
public void testCacheability() throws IOException {
826+
Directory directory = newDirectory();
827+
RandomIndexWriter iw = new RandomIndexWriter(random(), directory);
828+
iw.addDocument(new Document());
829+
final IndexSearcher searcher = new IndexSearcher(iw.getReader());
830+
iw.close();
831+
assertThat(searcher.getIndexReader().leaves().size(), greaterThan(0));
832+
818833
FunctionScoreQueryBuilder queryBuilder = createTestQueryBuilder();
819-
boolean isCacheable = isCacheable(queryBuilder);
820-
SearchExecutionContext context = createSearchExecutionContext();
834+
boolean requestCache = isCacheable(queryBuilder);
835+
SearchExecutionContext context = createSearchExecutionContext(searcher);
821836
QueryBuilder rewriteQuery = rewriteQuery(queryBuilder, new SearchExecutionContext(context));
822837
assertNotNull(rewriteQuery.toQuery(context));
823-
// we occasionally need to update the expected "isCacheable" flag after rewrite for MatchNoneQueryBuilder
838+
// we occasionally need to update the expected request cache flag after rewrite to MatchNoneQueryBuilder
824839
if (rewriteQuery instanceof MatchNoneQueryBuilder) {
825-
isCacheable = true;
840+
requestCache = true;
841+
}
842+
assertEquals("query should " + (requestCache ? "" : "not") + " be eligible for the request cache: " + queryBuilder.toString(),
843+
requestCache, context.isCacheable());
844+
845+
// test query cache
846+
if (rewriteQuery instanceof MatchNoneQueryBuilder == false) {
847+
Query luceneQuery = rewriteQuery.toQuery(context);
848+
Weight queryWeight = context.searcher().createWeight(searcher.rewrite(luceneQuery), ScoreMode.COMPLETE, 1.0f);
849+
for (LeafReaderContext ctx : context.getIndexReader().leaves()) {
850+
assertFalse(queryWeight.isCacheable(ctx));
851+
}
826852
}
827-
assertEquals("query should " + (isCacheable ? "" : "not") + " be cacheable: " + queryBuilder.toString(), isCacheable,
828-
context.isCacheable());
829853

830854
ScoreFunctionBuilder<?> scriptScoreFunction = new ScriptScoreFunctionBuilder(
831855
new Script(ScriptType.INLINE, MockScriptEngine.NAME, "1", Collections.emptyMap()));
832856
queryBuilder = new FunctionScoreQueryBuilder(new FilterFunctionBuilder[] {
833857
new FilterFunctionBuilder(RandomQueryBuilder.createQuery(random()), scriptScoreFunction) });
834-
context = createSearchExecutionContext();
858+
context = createSearchExecutionContext(searcher);
835859
rewriteQuery = rewriteQuery(queryBuilder, new SearchExecutionContext(context));
836860
assertNotNull(rewriteQuery.toQuery(context));
837-
assertTrue("function script query should be cacheable" + queryBuilder.toString(), context.isCacheable());
861+
assertTrue("function script query should be eligible for the request cache: " + queryBuilder.toString(),
862+
context.isCacheable());
863+
864+
// test query cache
865+
if (rewriteQuery instanceof MatchNoneQueryBuilder == false) {
866+
Query luceneQuery = rewriteQuery.toQuery(context);
867+
Weight queryWeight = context.searcher().createWeight(searcher.rewrite(luceneQuery), ScoreMode.COMPLETE, 1.0f);
868+
for (LeafReaderContext ctx : context.getIndexReader().leaves()) {
869+
assertFalse(queryWeight.isCacheable(ctx));
870+
}
871+
}
838872

839873
RandomScoreFunctionBuilder randomScoreFunctionBuilder = new RandomScoreFunctionBuilderWithFixedSeed();
840874
queryBuilder = new FunctionScoreQueryBuilder(new FilterFunctionBuilder[] {
841875
new FilterFunctionBuilder(RandomQueryBuilder.createQuery(random()), randomScoreFunctionBuilder) });
842-
context = createSearchExecutionContext();
876+
context = createSearchExecutionContext(searcher);
843877
rewriteQuery = rewriteQuery(queryBuilder, new SearchExecutionContext(context));
844878
assertNotNull(rewriteQuery.toQuery(context));
845-
assertFalse("function random query should not be cacheable: " + queryBuilder.toString(), context.isCacheable());
879+
assertFalse("function random query should not be eligible for the request cache: " + queryBuilder.toString(),
880+
context.isCacheable());
881+
882+
// test query cache
883+
if (rewriteQuery instanceof MatchNoneQueryBuilder == false) {
884+
Query luceneQuery = rewriteQuery.toQuery(context);
885+
Weight queryWeight = context.searcher().createWeight(searcher.rewrite(luceneQuery), ScoreMode.COMPLETE, 1.0f);
886+
for (LeafReaderContext ctx : context.getIndexReader().leaves()) {
887+
assertFalse(queryWeight.isCacheable(ctx));
888+
}
889+
}
890+
searcher.getIndexReader().close();
891+
directory.close();
846892
}
847893

848894
private boolean isCacheable(FunctionScoreQueryBuilder queryBuilder) {

0 commit comments

Comments
 (0)