Skip to content

Commit

Permalink
Scripting: ScriptFactory not required by compile (elastic#50344)
Browse files Browse the repository at this point in the history
Avoid backwards incompatible changes for 8.x and 7.6 by removing type
restriction on compile and Factory.  Factories may optionally implement
ScriptFactory.  If so, then they can indicate determinism and thus
cacheability.

Relates: elastic#49466
  • Loading branch information
stu-elastic committed Dec 19, 2019
1 parent 9033052 commit 4249af9
Show file tree
Hide file tree
Showing 33 changed files with 162 additions and 107 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.apache.lucene.analysis.tokenattributes.TypeAttribute;
import org.apache.lucene.util.AttributeSource;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptFactory;

/**
* A predicate based on the current token in a TokenStream
Expand Down Expand Up @@ -108,7 +107,7 @@ public boolean isKeyword() {
*/
public abstract boolean execute(Token token);

public interface Factory extends ScriptFactory {
public interface Factory {
AnalysisPredicateScript newInstance();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.test.ESTokenStreamTestCase;
import org.elasticsearch.test.IndexSettingsModule;
Expand Down Expand Up @@ -64,7 +63,7 @@ public boolean execute(Token token) {
@SuppressWarnings("unchecked")
ScriptService scriptService = new ScriptService(indexSettings, Collections.emptyMap(), Collections.emptyMap()){
@Override
public <FactoryType extends ScriptFactory> FactoryType compile(Script script, ScriptContext<FactoryType> context) {
public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryType> context) {
assertEquals(context, AnalysisPredicateScript.CONTEXT);
assertEquals(new Script("my_script"), script);
return (FactoryType) factory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.test.ESTokenStreamTestCase;
import org.elasticsearch.test.IndexSettingsModule;
Expand Down Expand Up @@ -64,7 +63,7 @@ public boolean execute(Token token) {
@SuppressWarnings("unchecked")
ScriptService scriptService = new ScriptService(indexSettings, Collections.emptyMap(), Collections.emptyMap()){
@Override
public <FactoryType extends ScriptFactory> FactoryType compile(Script script, ScriptContext<FactoryType> context) {
public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryType> context) {
assertEquals(context, AnalysisPredicateScript.CONTEXT);
assertEquals(new Script("token.getPosition() > 1"), script);
return (FactoryType) factory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptEngine;
import org.elasticsearch.script.ScriptException;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.script.TermsSetQueryScript;
import org.elasticsearch.search.lookup.SearchLookup;

Expand Down Expand Up @@ -88,33 +87,83 @@ public boolean execute() {
return wrappedFactory; });

contexts.put(FilterScript.CONTEXT,
(Expression expr) -> (FilterScript.Factory) (p, lookup) -> newFilterScript(expr, lookup, p));
(Expression expr) -> new FilterScript.Factory() {
@Override
public boolean isResultDeterministic() {
return true;
}

@Override
public FilterScript.LeafFactory newFactory(Map<String, Object> params, SearchLookup lookup) {
return newFilterScript(expr, lookup, params);
}
});

contexts.put(ScoreScript.CONTEXT,
(Expression expr) -> (ScoreScript.Factory) (p, lookup) -> newScoreScript(expr, lookup, p));
(Expression expr) -> new ScoreScript.Factory() {
@Override
public ScoreScript.LeafFactory newFactory(Map<String, Object> params, SearchLookup lookup) {
return newScoreScript(expr, lookup, params);
}

@Override
public boolean isResultDeterministic() {
return true;
}
});

contexts.put(TermsSetQueryScript.CONTEXT,
(Expression expr) -> (TermsSetQueryScript.Factory) (p, lookup) -> newTermsSetQueryScript(expr, lookup, p));

contexts.put(AggregationScript.CONTEXT,
(Expression expr) -> (AggregationScript.Factory) (p, lookup) -> newAggregationScript(expr, lookup, p));
(Expression expr) -> new AggregationScript.Factory() {
@Override
public AggregationScript.LeafFactory newFactory(Map<String, Object> params, SearchLookup lookup) {
return newAggregationScript(expr, lookup, params);
}

@Override
public boolean isResultDeterministic() {
return true;
}
});

contexts.put(NumberSortScript.CONTEXT,
(Expression expr) -> (NumberSortScript.Factory) (p, lookup) -> newSortScript(expr, lookup, p));
(Expression expr) -> new NumberSortScript.Factory() {
@Override
public NumberSortScript.LeafFactory newFactory(Map<String, Object> params, SearchLookup lookup) {
return newSortScript(expr, lookup, params);
}

@Override
public boolean isResultDeterministic() {
return true;
}
});

contexts.put(FieldScript.CONTEXT,
(Expression expr) -> (FieldScript.Factory) (p, lookup) -> newFieldScript(expr, lookup, p));
(Expression expr) -> new FieldScript.Factory() {
@Override
public FieldScript.LeafFactory newFactory(Map<String, Object> params, SearchLookup lookup) {
return newFieldScript(expr, lookup, params);
}

ExpressionScriptEngine.contexts = Collections.unmodifiableMap(contexts);
}
@Override
public boolean isResultDeterministic() {
return true;
}
});

ExpressionScriptEngine.contexts = Collections.unmodifiableMap(contexts);
}

@Override
public String getType() {
return NAME;
}

@Override
public <T extends ScriptFactory> T compile(
public <T> T compile(
String scriptName,
String scriptSource,
ScriptContext<T> context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@
import com.github.mustachejava.Mustache;
import com.github.mustachejava.MustacheException;
import com.github.mustachejava.MustacheFactory;

import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.SpecialPermission;
Expand All @@ -32,7 +31,6 @@
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptEngine;
import org.elasticsearch.script.ScriptException;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.script.TemplateScript;

import java.io.Reader;
Expand Down Expand Up @@ -65,7 +63,7 @@ public final class MustacheScriptEngine implements ScriptEngine {
* @return a compiled template object for later execution.
* */
@Override
public <T extends ScriptFactory> T compile(
public <T> T compile(
String templateName,
String templateSource,
ScriptContext<T> context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptEngine;
import org.elasticsearch.script.ScriptException;
import org.elasticsearch.script.ScriptFactory;
import org.objectweb.asm.ClassWriter;
import org.objectweb.asm.Opcodes;
import org.objectweb.asm.Type;
Expand Down Expand Up @@ -123,7 +122,7 @@ public String getType() {
}

@Override
public <T extends ScriptFactory> T compile(
public <T> T compile(
String scriptName,
String scriptSource,
ScriptContext<T> context,
Expand Down Expand Up @@ -169,7 +168,7 @@ public Set<ScriptContext<?>> getSupportedContexts() {
* @param <T> The factory class.
* @return A factory class that will return script instances.
*/
private <T extends ScriptFactory> Type generateStatefulFactory(
private <T> Type generateStatefulFactory(
Loader loader,
ScriptContext<T> context,
Set<String> extractedVariables
Expand Down Expand Up @@ -275,7 +274,7 @@ private <T extends ScriptFactory> Type generateStatefulFactory(
* @param <T> The factory class.
* @return A factory class that will return script instances.
*/
private <T extends ScriptFactory> T generateFactory(
private <T> T generateFactory(
Loader loader,
ScriptContext<T> context,
Set<String> extractedVariables,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
import org.elasticsearch.script.ScoreScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.ScriptType;
import org.elasticsearch.threadpool.ThreadPool;
Expand Down Expand Up @@ -435,7 +434,7 @@ public Map<String, Object> getParams() {

public abstract Object execute();

public interface Factory extends ScriptFactory {
public interface Factory {

PainlessTestScript newInstance(Map<String, Object> params);

Expand Down
Loading

0 comments on commit 4249af9

Please sign in to comment.