From 19b07ae4a25c35577defca57e5672f8e7c2f1be8 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 24 Oct 2018 15:11:25 -0700 Subject: [PATCH 1/3] Scripting: Add back lookup vars in score script The lookup vars under params (namely _fields and _source) were inadvertently removed when scoring scripts were converted to using script contexts. This commit adds them back, along with deprecation warnings for those that should not be used. --- .../org/elasticsearch/script/ScoreScript.java | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/script/ScoreScript.java b/server/src/main/java/org/elasticsearch/script/ScoreScript.java index 5c533298cbe13..23f616e42da30 100644 --- a/server/src/main/java/org/elasticsearch/script/ScoreScript.java +++ b/server/src/main/java/org/elasticsearch/script/ScoreScript.java @@ -26,6 +26,8 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.function.DoubleSupplier; @@ -34,6 +36,22 @@ */ public abstract class ScoreScript { + private static final Map DEPRECATIONS; + static { + Map deprecations = new HashMap<>(); + deprecations.put( + "doc", + "Accessing variable [doc] via [params.doc] from within a score script " + + "is deprecated in favor of directly accessing [doc]." + ); + deprecations.put( + "_doc", + "Accessing variable [doc] via [params._doc] from within a score script " + + "is deprecated in favor of directly accessing [doc]." + ); + DEPRECATIONS = Collections.unmodifiableMap(deprecations); + } + public static final String[] PARAMETERS = new String[]{}; /** The generic runtime parameters for the script. */ @@ -45,9 +63,13 @@ public abstract class ScoreScript { private DoubleSupplier scoreSupplier = () -> 0.0; public ScoreScript(Map params, SearchLookup lookup, LeafReaderContext leafContext) { - this.params = params; // null check needed b/c of expression engine subclass this.leafLookup = lookup == null ? null : lookup.getLeafSearchLookup(leafContext); + params = new HashMap<>(params); + if (leafLookup != null) { + params.putAll(leafLookup.asMap()); + } + this.params = new ParameterMap(params, DEPRECATIONS); } public abstract double execute(); From e064cd1e347080867d8ec90def72cf2742246f90 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 29 Oct 2018 21:13:15 -0700 Subject: [PATCH 2/3] give scorescript a special ctor for expressions --- .../script/expression/ExpressionScoreScript.java | 2 +- .../java/org/elasticsearch/script/ScoreScript.java | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScoreScript.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScoreScript.java index 120e8a9cabf7a..98a4b26ce1c6d 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScoreScript.java +++ b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScoreScript.java @@ -53,7 +53,7 @@ public boolean needs_score() { @Override public ScoreScript newInstance(final LeafReaderContext leaf) throws IOException { - return new ScoreScript(null, null, null) { + return new ScoreScript() { // Fake the scorer until setScorer is called. DoubleValues values = source.getValues(leaf, new DoubleValues() { @Override diff --git a/server/src/main/java/org/elasticsearch/script/ScoreScript.java b/server/src/main/java/org/elasticsearch/script/ScoreScript.java index 23f616e42da30..5bb864d942576 100644 --- a/server/src/main/java/org/elasticsearch/script/ScoreScript.java +++ b/server/src/main/java/org/elasticsearch/script/ScoreScript.java @@ -64,14 +64,18 @@ public abstract class ScoreScript { public ScoreScript(Map params, SearchLookup lookup, LeafReaderContext leafContext) { // null check needed b/c of expression engine subclass - this.leafLookup = lookup == null ? null : lookup.getLeafSearchLookup(leafContext); + this.leafLookup = lookup.getLeafSearchLookup(leafContext); params = new HashMap<>(params); - if (leafLookup != null) { - params.putAll(leafLookup.asMap()); - } + params.putAll(leafLookup.asMap()); this.params = new ParameterMap(params, DEPRECATIONS); } + // this constructor is special for expressions, which only wants to override execute + public ScoreScript() { + this.params = null; + this.leafLookup = null; + } + public abstract double execute(); /** Return the parameters for this script. */ From 66c04dea2fe1bced3da5b459c02e19f434aadb24 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 31 Oct 2018 14:04:14 -0700 Subject: [PATCH 3/3] move back to single ctor, special case for expression --- .../expression/ExpressionScoreScript.java | 2 +- .../org/elasticsearch/script/ScoreScript.java | 21 ++++++++++--------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScoreScript.java b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScoreScript.java index 98a4b26ce1c6d..120e8a9cabf7a 100644 --- a/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScoreScript.java +++ b/modules/lang-expression/src/main/java/org/elasticsearch/script/expression/ExpressionScoreScript.java @@ -53,7 +53,7 @@ public boolean needs_score() { @Override public ScoreScript newInstance(final LeafReaderContext leaf) throws IOException { - return new ScoreScript() { + return new ScoreScript(null, null, null) { // Fake the scorer until setScorer is called. DoubleValues values = source.getValues(leaf, new DoubleValues() { @Override diff --git a/server/src/main/java/org/elasticsearch/script/ScoreScript.java b/server/src/main/java/org/elasticsearch/script/ScoreScript.java index 5bb864d942576..c88c68fd407a2 100644 --- a/server/src/main/java/org/elasticsearch/script/ScoreScript.java +++ b/server/src/main/java/org/elasticsearch/script/ScoreScript.java @@ -64,16 +64,17 @@ public abstract class ScoreScript { public ScoreScript(Map params, SearchLookup lookup, LeafReaderContext leafContext) { // null check needed b/c of expression engine subclass - this.leafLookup = lookup.getLeafSearchLookup(leafContext); - params = new HashMap<>(params); - params.putAll(leafLookup.asMap()); - this.params = new ParameterMap(params, DEPRECATIONS); - } - - // this constructor is special for expressions, which only wants to override execute - public ScoreScript() { - this.params = null; - this.leafLookup = null; + if (lookup == null) { + assert params == null; + assert leafContext == null; + this.params = null; + this.leafLookup = null; + } else { + this.leafLookup = lookup.getLeafSearchLookup(leafContext); + params = new HashMap<>(params); + params.putAll(leafLookup.asMap()); + this.params = new ParameterMap(params, DEPRECATIONS); + } } public abstract double execute();