fix random score function builder to deal with empty seeds

This commit is contained in:
Simon Willnauer 2016-10-05 10:45:14 +02:00
parent 764a5fbb37
commit 7ba22bb75b
10 changed files with 45 additions and 30 deletions

View File

@ -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<Boolean> 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 {

View File

@ -337,28 +337,24 @@ public class QueryShardContext extends QueryRewriteContext {
public SearchScript getSearchScript(Script script, ScriptContext context, Map<String, String> params) {
failIfExecutionMode();
markAsNotCachable();
return scriptService.search(lookup(), script, context, params);
}
public Function<Map<String, Object>, SearchScript> getLazySearchScript(Script script, ScriptContext context,
Map<String, String> 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<String, String> params) {
failIfExecutionMode();
markAsNotCachable();
return scriptService.executable(script, context, params);
}
public Function<Map<String, Object>, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context,
Map<String, String> params) {
failIfExecutionMode();
markAsNotCachable();
CompiledScript executable = scriptService.compile(script, context, params);
return (p) -> scriptService.executable(executable, p);
}

View File

@ -49,12 +49,19 @@ public class RandomScoreFunctionBuilder extends ScoreFunctionBuilder<RandomScore
*/
public RandomScoreFunctionBuilder(StreamInput in) throws IOException {
super(in);
seed = in.readInt();
if (in.readBoolean()) {
seed = in.readInt();
}
}
@Override
protected void doWriteTo(StreamOutput out) throws IOException {
out.writeInt(seed);
if (seed != null) {
out.writeBoolean(true);
out.writeInt(seed);
} else {
out.writeBoolean(false);
}
}
@Override

View File

@ -42,7 +42,6 @@ import org.elasticsearch.index.query.ParsedQuery;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.shard.IndexShard;
import org.elasticsearch.index.similarity.SimilarityService;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.search.SearchExtBuilder;
import org.elasticsearch.search.SearchShardTarget;
import org.elasticsearch.search.aggregations.SearchContextAggregations;
@ -165,7 +164,7 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas
}
public final void resetCanCache() {
getQueryShardContext().setCachabe(true);
getQueryShardContext().setCachable(true);
}
public abstract ScrollContext scrollContext();

View File

@ -46,9 +46,10 @@ public class QueryShardContextTests extends ESTestCase {
IndexSettings indexSettings = new IndexSettings(indexMetadata.build(), Settings.EMPTY);
MapperService mapperService = mock(MapperService.class);
when(mapperService.getIndexSettings()).thenReturn(indexSettings);
final long nowInMillis = randomPositiveLong();
QueryShardContext context = new QueryShardContext(
indexSettings, null, null, mapperService, null, null, null, null, null, null,
System::currentTimeMillis);
() -> nowInMillis);
context.setAllowUnmappedFields(false);
MappedFieldType fieldType = new TextFieldMapper.TextFieldType();

View File

@ -172,12 +172,14 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
break;
case 3:
RandomScoreFunctionBuilder randomScoreFunctionBuilder = new RandomScoreFunctionBuilderWithFixedSeed();
if (randomBoolean()) {
randomScoreFunctionBuilder.seed(randomLong());
} else if (randomBoolean()) {
randomScoreFunctionBuilder.seed(randomInt());
} else {
randomScoreFunctionBuilder.seed(randomAsciiOfLengthBetween(1, 10));
if (randomBoolean()) { // sometimes provide no seed
if (randomBoolean()) {
randomScoreFunctionBuilder.seed(randomLong());
} else if (randomBoolean()) {
randomScoreFunctionBuilder.seed(randomInt());
} else {
randomScoreFunctionBuilder.seed(randomAsciiOfLengthBetween(1, 10));
}
}
functionBuilder = randomScoreFunctionBuilder;
break;
@ -777,7 +779,9 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
throws IOException, ParsingException {
RandomScoreFunctionBuilder builder = RandomScoreFunctionBuilder.fromXContent(parseContext);
RandomScoreFunctionBuilderWithFixedSeed replacement = new RandomScoreFunctionBuilderWithFixedSeed();
replacement.seed(builder.getSeed());
if (builder.getSeed() != null) {
replacement.seed(builder.getSeed());
}
return replacement;
}
}
@ -796,6 +800,9 @@ public class FunctionScoreQueryBuilderTests extends AbstractQueryTestCase<Functi
for (FilterFunctionBuilder builder : filterFunctionBuilders) {
if (builder.getScoreFunction() instanceof ScriptScoreFunctionBuilder) {
return false;
} else if (builder.getScoreFunction() instanceof RandomScoreFunctionBuilder
&& ((RandomScoreFunctionBuilder) builder.getScoreFunction()).getSeed() == null) {
return false;
}
}
return true;

View File

@ -156,12 +156,13 @@ public class QueryRescoreBuilderTests extends ESTestCase {
* than the test builder
*/
public void testBuildRescoreSearchContext() throws ElasticsearchParseException, IOException {
final long nowInMillis = randomPositiveLong();
Settings indexSettings = Settings.builder()
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).build();
IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(randomAsciiOfLengthBetween(1, 10), indexSettings);
// shard context will only need indicesQueriesRegistry for building Query objects nested in query rescorer
QueryShardContext mockShardContext = new QueryShardContext(idxSettings, null, null, null, null, null, indicesQueriesRegistry,
null, null, null, System::currentTimeMillis) {
null, null, null, () -> nowInMillis) {
@Override
public MappedFieldType fieldMapper(String name) {
TextFieldMapper.Builder builder = new TextFieldMapper.Builder(name);

View File

@ -234,8 +234,9 @@ public abstract class AbstractSortTestCase<T extends SortBuilder<T>> 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);

View File

@ -580,11 +580,17 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
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<QB extends AbstractQueryBuilder<QB>>
}
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<QB extends AbstractQueryBuilder<QB>>
private final BitsetFilterCache bitsetFilterCache;
private final ScriptService scriptService;
private final Client client;
private final long nowInMillis = randomPositiveLong();
ServiceHolder(Settings nodeSettings, Settings indexSettings,
Collection<Class<? extends Plugin>> plugins, AbstractQueryTestCase<?> testCase) throws IOException {
@ -1110,7 +1109,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
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<ScriptPlugin> scriptPlugins) {

View File

@ -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);