From 45d0dc54c5335436a284e573449537f3c8c0921a Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Fri, 17 Jul 2020 16:48:23 +0200 Subject: [PATCH] Pass script params through to fielddata impl (#59762) This addresses a TODO around using the script params, which are now parsed from the mappings. It also expand existing tests to verify that params are carried around and accessible in script for both fielddata and queries. Relates to #59332 --- .../fielddata/ScriptBinaryFieldData.java | 20 +++++++++----- .../mapper/RuntimeKeywordMappedFieldType.java | 2 +- .../RuntimeKeywordMappedFieldTypeTests.java | 26 ++++++++++++++----- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/fielddata/ScriptBinaryFieldData.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/fielddata/ScriptBinaryFieldData.java index 3b17f0b4f9f62..592006d6ca9d8 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/fielddata/ScriptBinaryFieldData.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/fielddata/ScriptBinaryFieldData.java @@ -23,6 +23,7 @@ import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.indices.breaker.CircuitBreakerService; +import org.elasticsearch.script.Script; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.MultiValueMode; import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; @@ -34,7 +35,6 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.List; public final class ScriptBinaryFieldData extends AbstractIndexComponent @@ -44,9 +44,11 @@ public final class ScriptBinaryFieldData extends AbstractIndexComponent public static class Builder implements IndexFieldData.Builder { + private final Script script; private final StringScriptFieldScript.Factory scriptFactory; - public Builder(StringScriptFieldScript.Factory scriptFactory) { + public Builder(Script script, StringScriptFieldScript.Factory scriptFactory) { + this.script = script; this.scriptFactory = scriptFactory; } @@ -58,23 +60,29 @@ public ScriptBinaryFieldData build( CircuitBreakerService breakerService, MapperService mapperService ) { - return new ScriptBinaryFieldData(indexSettings, fieldType.name(), scriptFactory); + return new ScriptBinaryFieldData(indexSettings, fieldType.name(), script, scriptFactory); } } private final String fieldName; + private final Script script; private final StringScriptFieldScript.Factory scriptFactory; private final SetOnce leafFactory = new SetOnce<>(); - private ScriptBinaryFieldData(IndexSettings indexSettings, String fieldName, StringScriptFieldScript.Factory scriptFactory) { + private ScriptBinaryFieldData( + IndexSettings indexSettings, + String fieldName, + Script script, + StringScriptFieldScript.Factory scriptFactory + ) { super(indexSettings); this.fieldName = fieldName; + this.script = script; this.scriptFactory = scriptFactory; } public void setSearchLookup(SearchLookup searchLookup) { - // TODO wire the params from the mappings definition, we don't parse them yet - this.leafFactory.set(scriptFactory.newFactory(Collections.emptyMap(), searchLookup)); + this.leafFactory.set(scriptFactory.newFactory(script.getParams(), searchLookup)); } @Override diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeKeywordMappedFieldType.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeKeywordMappedFieldType.java index a51e1a319a84f..c940502ecb736 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeKeywordMappedFieldType.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeKeywordMappedFieldType.java @@ -86,7 +86,7 @@ public boolean isAggregatable() { @Override public ScriptBinaryFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) { // TODO once we get SearchLookup as an argument, we can already call scriptFactory.newFactory here and pass through the result - return new ScriptBinaryFieldData.Builder(scriptFactory); + return new ScriptBinaryFieldData.Builder(script, scriptFactory); } private StringScriptFieldScript.LeafFactory leafFactory(QueryShardContext context) { diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeKeywordMappedFieldTypeTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeKeywordMappedFieldTypeTests.java index c5baa0b7ff4d9..ee316c85ce34c 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeKeywordMappedFieldTypeTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeKeywordMappedFieldTypeTests.java @@ -29,10 +29,12 @@ import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.query.QueryShardContext; import org.elasticsearch.painless.PainlessPlugin; +import org.elasticsearch.painless.PainlessScriptEngine; import org.elasticsearch.plugins.ExtensiblePlugin.ExtensionLoader; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptModule; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.lookup.SearchLookup; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.runtimefields.RuntimeFields; @@ -43,6 +45,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.function.BiConsumer; import static java.util.Collections.emptyMap; @@ -58,7 +61,10 @@ public void testDocValues() throws IOException { List results = new ArrayList<>(); try (DirectoryReader reader = iw.getReader()) { IndexSearcher searcher = newSearcher(reader); - RuntimeKeywordMappedFieldType ft = build("for (def v : source.foo) {value(v.toString())}"); + RuntimeKeywordMappedFieldType ft = build( + "for (def v : source.foo) {value(v.toString() + params.param)}", + Map.of("param", "-suffix") + ); IndexMetadata imd = IndexMetadata.builder("test") .settings(Settings.builder().put("index.version.created", Version.CURRENT)) .numberOfShards(1) @@ -73,11 +79,11 @@ public ScoreMode scoreMode() { } @Override - public LeafCollector getLeafCollector(LeafReaderContext context) throws IOException { + public LeafCollector getLeafCollector(LeafReaderContext context) { SortedBinaryDocValues dv = ifd.load(context).getBytesValues(); return new LeafCollector() { @Override - public void setScorer(Scorable scorer) throws IOException {} + public void setScorer(Scorable scorer) {} @Override public void collect(int doc) throws IOException { @@ -90,7 +96,7 @@ public void collect(int doc) throws IOException { }; } }); - assertThat(results, equalTo(List.of("1", "1", "2"))); + assertThat(results, equalTo(List.of("1-suffix", "1-suffix", "2-suffix"))); } } } @@ -213,7 +219,8 @@ public void testTermQuery() throws IOException { iw.addDocument(List.of(new StoredField("_source", new BytesRef("{\"foo\": 2}")))); try (DirectoryReader reader = iw.getReader()) { IndexSearcher searcher = newSearcher(reader); - assertThat(searcher.count(build("value(source.foo.toString())").termQuery("1", mockContext())), equalTo(1)); + RuntimeKeywordMappedFieldType fieldType = build("value(source.foo.toString() + params.param)", Map.of("param", "-suffix")); + assertThat(searcher.count(fieldType.termQuery("1-suffix", mockContext())), equalTo(1)); } } } @@ -255,7 +262,14 @@ public void testWildcardQueryIsExpensive() throws IOException { } private RuntimeKeywordMappedFieldType build(String code) throws IOException { - Script script = new Script(code); + return build(new Script(code)); + } + + private RuntimeKeywordMappedFieldType build(String code, Map params) throws IOException { + return build(new Script(ScriptType.INLINE, PainlessScriptEngine.NAME, code, params)); + } + + private RuntimeKeywordMappedFieldType build(Script script) throws IOException { PainlessPlugin painlessPlugin = new PainlessPlugin(); painlessPlugin.loadExtensions(new ExtensionLoader() { @Override