From 7ba22bb75bd54e2a70938f686a17d63cea950228 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 5 Oct 2016 10:45:14 +0200 Subject: [PATCH] fix random score function builder to deal with empty seeds --- .../index/query/QueryRewriteContext.java | 7 +++++-- .../index/query/QueryShardContext.java | 4 ---- .../RandomScoreFunctionBuilder.java | 11 ++++++++-- .../search/internal/SearchContext.java | 3 +-- .../index/query/QueryShardContextTests.java | 3 ++- .../FunctionScoreQueryBuilderTests.java | 21 ++++++++++++------- .../rescore/QueryRescoreBuilderTests.java | 3 ++- .../search/sort/AbstractSortTestCase.java | 3 ++- .../test/AbstractQueryTestCase.java | 17 +++++++-------- .../search/MockSearchServiceTests.java | 3 ++- 10 files changed, 45 insertions(+), 30 deletions(-) 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 bc3c6ab52ab..48e9d134a77 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -48,7 +48,7 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { protected final Client client; protected final IndexReader reader; protected final ClusterState clusterState; - protected boolean cachable; + protected boolean cachable = true; private final SetOnce executionMode = new SetOnce<>(); public QueryRewriteContext(IndexSettings indexSettings, MapperService mapperService, ScriptService scriptService, @@ -127,7 +127,9 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { return cachable; } - public void setCachabe(boolean cachabe) { this.cachable = cachabe; } + public void setCachable(boolean cachabe) { + this.cachable = cachabe; + } public BytesReference getTemplateBytes(Script template) { failIfExecutionMode(); @@ -141,6 +143,7 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { } protected void failIfExecutionMode() { + markAsNotCachable(); if (executionMode.get() == Boolean.TRUE) { throw new IllegalArgumentException("features that prevent cachability are disabled on this context"); } else { diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 200d5fde0c7..3869ecfd2c1 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -337,28 +337,24 @@ public class QueryShardContext extends QueryRewriteContext { public SearchScript getSearchScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); - markAsNotCachable(); return scriptService.search(lookup(), script, context, params); } public Function, SearchScript> getLazySearchScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); - markAsNotCachable(); CompiledScript compile = scriptService.compile(script, context, params); return (p) -> scriptService.search(lookup(), compile, p); } public ExecutableScript getExecutableScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); - markAsNotCachable(); return scriptService.executable(script, context, params); } public Function, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context, Map params) { failIfExecutionMode(); - markAsNotCachable(); CompiledScript executable = scriptService.compile(script, context, params); return (p) -> scriptService.executable(executable, p); } diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/RandomScoreFunctionBuilder.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/RandomScoreFunctionBuilder.java index 4f4a3b54a21..f630c0c9b76 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/RandomScoreFunctionBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/RandomScoreFunctionBuilder.java @@ -49,12 +49,19 @@ public class RandomScoreFunctionBuilder extends ScoreFunctionBuilder nowInMillis); context.setAllowUnmappedFields(false); MappedFieldType fieldType = new TextFieldMapper.TextFieldType(); diff --git a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java index bbc7d945046..a799a5af6a9 100644 --- a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java @@ -172,12 +172,14 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase nowInMillis) { @Override public MappedFieldType fieldMapper(String name) { TextFieldMapper.Builder builder = new TextFieldMapper.Builder(name); diff --git a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java index 6f45a320fbc..6293401ebdd 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java +++ b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java @@ -234,8 +234,9 @@ public abstract class AbstractSortTestCase> extends EST public void onCache(ShardId shardId, Accountable accountable) { } }); + long nowInMillis = randomPositiveLong(); return new QueryShardContext(idxSettings, bitsetFilterCache, ifds, null, null, scriptService, - indicesQueriesRegistry, null, null, null, System::currentTimeMillis) { + indicesQueriesRegistry, null, null, null, () -> nowInMillis) { @Override public MappedFieldType fieldMapper(String name) { return provideMappedFieldType(name); diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index dd93615a27d..70915eeafa4 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -580,11 +580,17 @@ public abstract class AbstractQueryTestCase> public void testToQuery() throws IOException { for (int runs = 0; runs < NUMBER_OF_TESTQUERIES; runs++) { QueryShardContext context = createShardContext(); + assert context.isCachable(); context.setAllowUnmappedFields(true); QB firstQuery = createTestQueryBuilder(); QB controlQuery = copyQuery(firstQuery); setSearchContext(randomTypes, context); // only set search context for toQuery to be more realistic Query firstLuceneQuery = rewriteQuery(firstQuery, context).toQuery(context); + if (isCachable(firstQuery)) { + assert context.isCachable() : firstQuery.toString(); + } else { + assert context.isCachable() == false : firstQuery.toString(); + } assertNotNull("toQuery should not return null", firstLuceneQuery); assertLuceneQuery(firstQuery, firstLuceneQuery, context); //remove after assertLuceneQuery since the assertLuceneQuery impl might access the context as well @@ -630,15 +636,7 @@ public abstract class AbstractQueryTestCase> } private QueryBuilder rewriteQuery(QB queryBuilder, QueryRewriteContext rewriteContext) throws IOException { - final boolean wasCachable = rewriteContext.isCachable(); QueryBuilder rewritten = QueryBuilder.rewriteQuery(queryBuilder, rewriteContext); - if (wasCachable == true) { // it's not resettable so we can only check it once - if (isCachable(queryBuilder)) { - assert rewriteContext.isCachable() : queryBuilder.toString(); - } else { - assert rewriteContext.isCachable() == false; - } - } // extra safety to fail fast - serialize the rewritten version to ensure it's serializable. assertSerialization(rewritten); return rewritten; @@ -1032,6 +1030,7 @@ public abstract class AbstractQueryTestCase> private final BitsetFilterCache bitsetFilterCache; private final ScriptService scriptService; private final Client client; + private final long nowInMillis = randomPositiveLong(); ServiceHolder(Settings nodeSettings, Settings indexSettings, Collection> plugins, AbstractQueryTestCase testCase) throws IOException { @@ -1110,7 +1109,7 @@ public abstract class AbstractQueryTestCase> QueryShardContext createShardContext() { ClusterState state = ClusterState.builder(new ClusterName("_name")).build(); return new QueryShardContext(idxSettings, bitsetFilterCache, indexFieldDataService, mapperService, similarityService, - scriptService, indicesQueriesRegistry, this.client, null, state, System::currentTimeMillis); + scriptService, indicesQueriesRegistry, this.client, null, state, () -> nowInMillis); } ScriptModule createScriptModule(List scriptPlugins) { diff --git a/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java b/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java index f61a3a2b48e..8353a58e969 100644 --- a/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java +++ b/test/framework/src/test/java/org/elasticsearch/search/MockSearchServiceTests.java @@ -33,8 +33,9 @@ import org.elasticsearch.test.TestSearchContext; public class MockSearchServiceTests extends ESTestCase { public void testAssertNoInFlightContext() { + final long nowInMillis = randomPositiveLong(); SearchContext s = new TestSearchContext(new QueryShardContext(new IndexSettings(IndexMetaData.PROTO, Settings.EMPTY), null, null, - null, null, null, null, null, null, null, System::currentTimeMillis)) { + null, null, null, null, null, null, null, () -> nowInMillis)) { @Override public SearchShardTarget shardTarget() { return new SearchShardTarget("node", new Index("idx", "ignored"), 0);