From 620946d5e4f9cbbf0d80b846f816d67920f76613 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 24 Aug 2020 14:31:14 -0400 Subject: [PATCH 1/4] Convert double script to return array This replaces the value collection method in `double` valued runtime script fields with simply returning a `double[]`. Painless has some "convert" features that allow us to define automatic conversions from things like `double` and `Collection` into `double[]` so we use those. --- .../rest/CoreTestsWithRuntimeFieldsIT.java | 8 ++- .../AbstractLongScriptFieldScript.java | 2 + .../AbstractScriptFieldScript.java | 2 - .../BooleanScriptFieldScript.java | 2 + .../DoubleScriptFieldScript.java | 53 +++++++------------ .../runtimefields/IpScriptFieldScript.java | 2 + .../StringScriptFieldScript.java | 2 + .../fielddata/ScriptDoubleDocValues.java | 11 ++-- .../query/AbstractDoubleScriptFieldQuery.java | 5 +- .../query/DoubleScriptFieldExistsQuery.java | 4 +- .../query/DoubleScriptFieldRangeQuery.java | 6 +-- .../query/DoubleScriptFieldTermQuery.java | 6 +-- .../query/DoubleScriptFieldTermsQuery.java | 6 +-- .../xpack/runtimefields/double_whitelist.txt | 6 +-- .../DoubleScriptFieldScriptTests.java | 4 +- .../ScriptDoubleMappedFieldTypeTests.java | 22 +++++--- .../DoubleScriptFieldExistsQueryTests.java | 10 ++-- .../DoubleScriptFieldRangeQueryTests.java | 16 +++--- .../DoubleScriptFieldTermQueryTests.java | 7 ++- .../DoubleScriptFieldTermsQueryTests.java | 13 +++-- .../test/runtime_fields/30_double.yml | 36 +++++++++---- 21 files changed, 120 insertions(+), 103 deletions(-) diff --git a/x-pack/plugin/runtime-fields/qa/rest/src/yamlRestTest/java/org/elasticsearch/xpack/runtimefields/rest/CoreTestsWithRuntimeFieldsIT.java b/x-pack/plugin/runtime-fields/qa/rest/src/yamlRestTest/java/org/elasticsearch/xpack/runtimefields/rest/CoreTestsWithRuntimeFieldsIT.java index a987d51c789b2..d7af376e85436 100644 --- a/x-pack/plugin/runtime-fields/qa/rest/src/yamlRestTest/java/org/elasticsearch/xpack/runtimefields/rest/CoreTestsWithRuntimeFieldsIT.java +++ b/x-pack/plugin/runtime-fields/qa/rest/src/yamlRestTest/java/org/elasticsearch/xpack/runtimefields/rest/CoreTestsWithRuntimeFieldsIT.java @@ -166,6 +166,9 @@ private static String painlessToLoadFromSource(String name, String type) { return null; } StringBuilder b = new StringBuilder(); + if ("double".equals(type)) { + b.append("List result = new ArrayList();"); + } b.append("def v = source['").append(name).append("'];\n"); b.append("if (v instanceof Iterable) {\n"); b.append(" for (def vv : ((Iterable) v)) {\n"); @@ -180,6 +183,9 @@ private static String painlessToLoadFromSource(String name, String type) { b.append(" ").append(emit).append("\n"); b.append(" }\n"); b.append("}\n"); + if ("double".equals(type)) { + b.append("return result;"); + } return b.toString(); } @@ -188,7 +194,7 @@ private static String painlessToLoadFromSource(String name, String type) { Map.entry(DateFieldMapper.CONTENT_TYPE, "millis(parse(value.toString()));"), Map.entry( NumberType.DOUBLE.typeName(), - "value(value instanceof Number ? ((Number) value).doubleValue() : Double.parseDouble(value.toString()));" + "result.add(value instanceof Number ? ((Number) value).doubleValue() : Double.parseDouble(value.toString()));" ), Map.entry(KeywordFieldMapper.CONTENT_TYPE, "value(value.toString());"), Map.entry(IpFieldMapper.CONTENT_TYPE, "stringValue(value.toString());"), diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractLongScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractLongScriptFieldScript.java index 7d5c366538133..9bd72bee5ad62 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractLongScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractLongScriptFieldScript.java @@ -23,6 +23,8 @@ public AbstractLongScriptFieldScript(Map params, SearchLookup se super(params, searchLookup, ctx); } + public abstract void execute(); + /** * Execute the script for the provided {@code docId}. */ diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java index 67693c4faea83..e18f01ada2e1b 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/AbstractScriptFieldScript.java @@ -81,6 +81,4 @@ public final Map getSource() { public final Map> getDoc() { return leafSearchLookup.doc(); } - - public abstract void execute(); } diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/BooleanScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/BooleanScriptFieldScript.java index cc597cb8127d5..0cb70ae568de5 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/BooleanScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/BooleanScriptFieldScript.java @@ -42,6 +42,8 @@ public BooleanScriptFieldScript(Map params, SearchLookup searchL super(params, searchLookup, ctx); } + public abstract void execute(); + /** * Execute the script for the provided {@code docId}. */ diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java index cb3d4b48805d7..c0100cc02f74e 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.runtimefields; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.util.ArrayUtil; import org.elasticsearch.painless.spi.Whitelist; import org.elasticsearch.painless.spi.WhitelistLoader; import org.elasticsearch.script.ScriptContext; @@ -15,6 +14,7 @@ import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; +import java.util.Collection; import java.util.List; import java.util.Map; @@ -35,55 +35,38 @@ public interface LeafFactory { DoubleScriptFieldScript newInstance(LeafReaderContext ctx) throws IOException; } - private double[] values = new double[1]; - private int count; - public DoubleScriptFieldScript(Map params, SearchLookup searchLookup, LeafReaderContext ctx) { super(params, searchLookup, ctx); } + public abstract double[] execute(); + /** * Execute the script for the provided {@code docId}. */ - public final void runForDoc(int docId) { - count = 0; + public final double[] runForDoc(int docId) { setDocument(docId); - execute(); + return execute(); } - /** - * Values from the last time {@link #runForDoc(int)} was called. This array - * is mutable and will change with the next call of {@link #runForDoc(int)}. - * It is also oversized and will contain garbage at all indices at and - * above {@link #count()}. - */ - public final double[] values() { - return values; + public static double[] convertFromDouble(double v) { + return new double[] { v }; } - /** - * The number of results produced the last time {@link #runForDoc(int)} was called. - */ - public final int count() { - return count; - } - - private void collectValue(double v) { - if (values.length < count + 1) { - values = ArrayUtil.grow(values, count + 1); + public static double[] convertFromCollection(Collection v) { + double[] result = new double[v.size()]; + int i = 0; + for (Object o : v) { + result[i++] = ((Number) o).doubleValue(); } - values[count++] = v; + return result; } - public static class Value { - private final DoubleScriptFieldScript script; - - public Value(DoubleScriptFieldScript script) { - this.script = script; - } - - public void value(double v) { - script.collectValue(v); + public static double[] convertFromDef(Object o) { + if (o instanceof Double) { + return convertFromDouble(((Double) o).doubleValue()); + } else { + return (double[]) o; } } } diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/IpScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/IpScriptFieldScript.java index 5739e89a0c701..48e466f3be8e3 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/IpScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/IpScriptFieldScript.java @@ -63,6 +63,8 @@ public IpScriptFieldScript(Map params, SearchLookup searchLookup super(params, searchLookup, ctx); } + public abstract void execute(); + /** * Execute the script for the provided {@code docId}. */ diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/StringScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/StringScriptFieldScript.java index 8bb94783a8afc..2f4bac6617248 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/StringScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/StringScriptFieldScript.java @@ -41,6 +41,8 @@ public StringScriptFieldScript(Map params, SearchLookup searchLo super(params, searchLookup, ctx); } + public abstract void execute(); + /** * Execute the script for the provided {@code docId}. *

diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/fielddata/ScriptDoubleDocValues.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/fielddata/ScriptDoubleDocValues.java index 135229e260eba..08be6f54cd993 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/fielddata/ScriptDoubleDocValues.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/fielddata/ScriptDoubleDocValues.java @@ -14,6 +14,7 @@ public final class ScriptDoubleDocValues extends SortedNumericDoubleValues { private final DoubleScriptFieldScript script; + private double[] values; private int cursor; ScriptDoubleDocValues(DoubleScriptFieldScript script) { @@ -22,22 +23,22 @@ public final class ScriptDoubleDocValues extends SortedNumericDoubleValues { @Override public boolean advanceExact(int docId) { - script.runForDoc(docId); - if (script.count() == 0) { + values = script.runForDoc(docId); + if (values.length == 0) { return false; } - Arrays.sort(script.values(), 0, script.count()); + Arrays.sort(values); cursor = 0; return true; } @Override public double nextValue() throws IOException { - return script.values()[cursor++]; + return values[cursor++]; } @Override public int docValueCount() { - return script.count(); + return values.length; } } diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/AbstractDoubleScriptFieldQuery.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/AbstractDoubleScriptFieldQuery.java index 5c4841085e26f..a4d6d3cfcef75 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/AbstractDoubleScriptFieldQuery.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/AbstractDoubleScriptFieldQuery.java @@ -36,7 +36,7 @@ abstract class AbstractDoubleScriptFieldQuery extends AbstractScriptFieldQuery { /** * Does the value match this query? */ - protected abstract boolean matches(double[] values, int count); + protected abstract boolean matches(double[] values); @Override public final Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode, float boost) throws IOException { @@ -53,8 +53,7 @@ public Scorer scorer(LeafReaderContext ctx) throws IOException { TwoPhaseIterator twoPhase = new TwoPhaseIterator(approximation) { @Override public boolean matches() throws IOException { - script.runForDoc(approximation().docID()); - return AbstractDoubleScriptFieldQuery.this.matches(script.values(), script.count()); + return AbstractDoubleScriptFieldQuery.this.matches(script.runForDoc(approximation().docID())); } @Override diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldExistsQuery.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldExistsQuery.java index 620b6569457d1..6edfdb4633687 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldExistsQuery.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldExistsQuery.java @@ -15,8 +15,8 @@ public DoubleScriptFieldExistsQuery(Script script, DoubleScriptFieldScript.LeafF } @Override - protected boolean matches(double[] values, int count) { - return count > 0; + protected boolean matches(double[] values) { + return values.length > 0; } @Override diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQuery.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQuery.java index 8bb407d3902a6..52081dbb6897e 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQuery.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQuery.java @@ -29,9 +29,9 @@ public DoubleScriptFieldRangeQuery( } @Override - protected boolean matches(double[] values, int count) { - for (int i = 0; i < count; i++) { - if (lowerValue <= values[i] && values[i] <= upperValue) { + protected boolean matches(double[] values) { + for (double value : values) { + if (lowerValue <= value && value <= upperValue) { return true; } } diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermQuery.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermQuery.java index 2949aee788885..9da9691fc06e2 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermQuery.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermQuery.java @@ -20,9 +20,9 @@ public DoubleScriptFieldTermQuery(Script script, DoubleScriptFieldScript.LeafFac } @Override - protected boolean matches(double[] values, int count) { - for (int i = 0; i < count; i++) { - if (term == values[i]) { + protected boolean matches(double[] values) { + for (double value : values) { + if (term == value) { return true; } } diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermsQuery.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermsQuery.java index 990430807e783..30887765ca8dc 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermsQuery.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermsQuery.java @@ -29,9 +29,9 @@ public DoubleScriptFieldTermsQuery(Script script, DoubleScriptFieldScript.LeafFa } @Override - protected boolean matches(double[] values, int count) { - for (int i = 0; i < count; i++) { - if (terms.contains(Double.doubleToLongBits(values[i]))) { + protected boolean matches(double[] values) { + for (double value : values) { + if (terms.contains(Double.doubleToLongBits(value))) { return true; } } diff --git a/x-pack/plugin/runtime-fields/src/main/resources/org/elasticsearch/xpack/runtimefields/double_whitelist.txt b/x-pack/plugin/runtime-fields/src/main/resources/org/elasticsearch/xpack/runtimefields/double_whitelist.txt index ce8963b6f7cbf..ae243bf77600a 100644 --- a/x-pack/plugin/runtime-fields/src/main/resources/org/elasticsearch/xpack/runtimefields/double_whitelist.txt +++ b/x-pack/plugin/runtime-fields/src/main/resources/org/elasticsearch/xpack/runtimefields/double_whitelist.txt @@ -9,10 +9,6 @@ class org.elasticsearch.xpack.runtimefields.DoubleScriptFieldScript @no_import { } -static_import { - void value(org.elasticsearch.xpack.runtimefields.DoubleScriptFieldScript, double) bound_to org.elasticsearch.xpack.runtimefields.DoubleScriptFieldScript$Value -} - -# This import is required to make painless happy and it isn't 100% clear why +# This whitelist is required to allow painless to build the factory class org.elasticsearch.xpack.runtimefields.DoubleScriptFieldScript$Factory @no_import { } diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java index 274fb26cdc37a..3b1c698c1e6ee 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java @@ -15,8 +15,8 @@ public class DoubleScriptFieldScriptTests extends ScriptFieldScriptTestCase (ctx) -> new DoubleScriptFieldScript(params, lookup, ctx) { @Override - public void execute() { - for (Object foo : (List) getSource().get("foo")) { - new DoubleScriptFieldScript.Value(this).value(((Number) foo).doubleValue()); + public double[] execute() { + List foos = (List) getSource().get("foo"); + double[] results = new double[foos.size()]; + int i = 0; + for (Object foo : foos) { + results[i++] = ((Number) foo).doubleValue(); } + return results; } }; case "add_param": return (params, lookup) -> (ctx) -> new DoubleScriptFieldScript(params, lookup, ctx) { @Override - public void execute() { - for (Object foo : (List) getSource().get("foo")) { - new DoubleScriptFieldScript.Value(this).value( - ((Number) foo).doubleValue() + ((Number) getParams().get("param")).doubleValue() - ); + public double[] execute() { + List foos = (List) getSource().get("foo"); + double[] results = new double[foos.size()]; + int i = 0; + for (Object foo : foos) { + results[i++] = ((Number) getParams().get("param")).doubleValue() + ((Number) foo).doubleValue(); } + return results; } }; default: diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldExistsQueryTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldExistsQueryTests.java index f8889aa15ab5e..ccdee7fc2d5eb 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldExistsQueryTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldExistsQueryTests.java @@ -29,9 +29,13 @@ protected DoubleScriptFieldExistsQuery mutate(DoubleScriptFieldExistsQuery orig) @Override public void testMatches() { - assertTrue(createTestInstance().matches(new double[0], randomIntBetween(1, Integer.MAX_VALUE))); - assertFalse(createTestInstance().matches(new double[0], 0)); - assertFalse(createTestInstance().matches(new double[] { 1, 2, 3 }, 0)); + assertTrue(createTestInstance().matches(new double[] { 1 })); + assertFalse(createTestInstance().matches(new double[0])); + double[] big = new double[between(1, 10000)]; + for (int i = 0; i < big.length; i++) { + big[i] = 1.0; + } + assertTrue(createTestInstance().matches(big)); } @Override diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQueryTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQueryTests.java index e4ff8ee7c347f..92cb468dc6f70 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQueryTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQueryTests.java @@ -57,14 +57,14 @@ protected DoubleScriptFieldRangeQuery mutate(DoubleScriptFieldRangeQuery orig) { @Override public void testMatches() { DoubleScriptFieldRangeQuery query = new DoubleScriptFieldRangeQuery(randomScript(), leafFactory, "test", 1.2, 3.14); - assertTrue(query.matches(new double[] { 1.2 }, 1)); - assertTrue(query.matches(new double[] { 3.14 }, 1)); - assertTrue(query.matches(new double[] { 2 }, 1)); - assertFalse(query.matches(new double[] { 0 }, 0)); - assertFalse(query.matches(new double[] { 5 }, 1)); - assertTrue(query.matches(new double[] { 2, 5 }, 2)); - assertTrue(query.matches(new double[] { 5, 2 }, 2)); - assertFalse(query.matches(new double[] { 5, 2 }, 1)); + assertTrue(query.matches(new double[] { 1.2 })); + assertTrue(query.matches(new double[] { 3.14 })); + assertTrue(query.matches(new double[] { 2 })); + assertFalse(query.matches(new double[] {})); + assertFalse(query.matches(new double[] { 5 })); + assertTrue(query.matches(new double[] { 2, 5 })); + assertTrue(query.matches(new double[] { 5, 2 })); + assertFalse(query.matches(new double[] { 1 })); } @Override diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermQueryTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermQueryTests.java index 2fe5038bce413..b53a51ddeff69 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermQueryTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermQueryTests.java @@ -46,10 +46,9 @@ protected DoubleScriptFieldTermQuery mutate(DoubleScriptFieldTermQuery orig) { @Override public void testMatches() { DoubleScriptFieldTermQuery query = new DoubleScriptFieldTermQuery(randomScript(), leafFactory, "test", 3.14); - assertTrue(query.matches(new double[] { 3.14 }, 1)); // Match because value matches - assertFalse(query.matches(new double[] { 2 }, 1)); // No match because wrong value - assertFalse(query.matches(new double[] { 2, 3.14 }, 1)); // No match because value after count of values - assertTrue(query.matches(new double[] { 2, 3.14 }, 2)); // Match because one value matches + assertTrue(query.matches(new double[] { 3.14 })); + assertFalse(query.matches(new double[] { 2 })); + assertTrue(query.matches(new double[] { 2, 3.14 })); } @Override diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermsQueryTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermsQueryTests.java index d2d3a2f427e37..2039d1da282ef 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermsQueryTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldTermsQueryTests.java @@ -75,13 +75,12 @@ public void testMatches() { "test", LongHashSet.from(Double.doubleToLongBits(0.1), Double.doubleToLongBits(0.2), Double.doubleToLongBits(7.5)) ); - assertTrue(query.matches(new double[] { 0.1 }, 1)); - assertTrue(query.matches(new double[] { 0.2 }, 1)); - assertTrue(query.matches(new double[] { 7.5 }, 1)); - assertTrue(query.matches(new double[] { 0.1, 0 }, 2)); - assertTrue(query.matches(new double[] { 0, 0.1 }, 2)); - assertFalse(query.matches(new double[] { 0 }, 1)); - assertFalse(query.matches(new double[] { 0, 0.1 }, 1)); + assertTrue(query.matches(new double[] { 0.1 })); + assertTrue(query.matches(new double[] { 0.2 })); + assertTrue(query.matches(new double[] { 7.5 })); + assertTrue(query.matches(new double[] { 0.1, 0 })); + assertTrue(query.matches(new double[] { 0, 0.1 })); + assertFalse(query.matches(new double[] { 0 })); } @Override diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml index 6b745f8fe396e..1d4a596e7aa5d 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml @@ -22,33 +22,47 @@ setup: runtime_type: double script: source: | - for (double v : doc['voltage']) { - value(v / params.max); + def voltage = doc['voltage']; + double[] result = new double[voltage.size()]; + int i = 0; + for (double v : voltage) { + result[i++] = v / params.max; } + return result; params: max: 5.8 - # Test fetching from _source + # Test fetching from _source and the the def -> double[] conversion voltage_percent_from_source: type: runtime_script runtime_type: double script: source: | - value(source['voltage'] / params.max); + source['voltage'] / params.max; params: max: 5.8 - # Test fetching many values + # Test the implicit conversion from double -> double[] + voltage_percent_returning_double: + type: runtime_script + runtime_type: double + script: + source: | + ((double) source['voltage']) / 5.8; + # Test fetching many values and the Collection -> double[] conversion voltage_sqrts: type: runtime_script runtime_type: double script: source: | + List result = new ArrayList(); + int i = 0; for (double voltage : doc['voltage']) { double v = voltage; while (v > 1.2) { - value(v); + result.add(v); v = Math.sqrt(v); } } + return result; - do: bulk: @@ -77,9 +91,13 @@ setup: - match: {sensor.mappings.properties.voltage_percent.runtime_type: double } - match: sensor.mappings.properties.voltage_percent.script.source: | - for (double v : doc['voltage']) { - value(v / params.max); + def voltage = doc['voltage']; + double[] result = new double[voltage.size()]; + int i = 0; + for (double v : voltage) { + result[i++] = v / params.max; } + return result; - match: {sensor.mappings.properties.voltage_percent.script.params: {max: 5.8} } - match: {sensor.mappings.properties.voltage_percent.script.lang: painless } @@ -90,7 +108,7 @@ setup: index: sensor body: sort: timestamp - docvalue_fields: [voltage_percent, voltage_percent_from_source, voltage_sqrts] + docvalue_fields: [voltage_percent, voltage_percent_from_source, voltage_percent_returning_double, voltage_sqrts] - match: {hits.total.value: 6} - match: {hits.hits.0.fields.voltage_percent: [0.6896551724137931] } - match: {hits.hits.0.fields.voltage_percent_from_source: [0.6896551724137931] } From a34e57bf94a2185465ca36a99e0db2f31082ee10 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 25 Aug 2020 10:45:51 -0400 Subject: [PATCH 2/4] Unit tests for conversions --- .../DoubleScriptFieldScript.java | 2 ++ .../DoubleScriptFieldScriptTests.java | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java index c0100cc02f74e..a97175e401648 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java @@ -65,6 +65,8 @@ public static double[] convertFromCollection(Collection v) { public static double[] convertFromDef(Object o) { if (o instanceof Double) { return convertFromDouble(((Double) o).doubleValue()); + } else if (o instanceof Collection) { + return convertFromCollection((Collection) o); } else { return (double[]) o; } diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java index 3b1c698c1e6ee..7bfefc16dbd2f 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java @@ -8,6 +8,10 @@ import org.elasticsearch.script.ScriptContext; +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; + public class DoubleScriptFieldScriptTests extends ScriptFieldScriptTestCase { public static final DoubleScriptFieldScript.Factory DUMMY = (params, lookup) -> ctx -> new DoubleScriptFieldScript( params, @@ -29,4 +33,23 @@ protected ScriptContext context() { protected DoubleScriptFieldScript.Factory dummyScript() { return DUMMY; } + + public void testConvertDouble() { + double v = randomDouble(); + assertThat(DoubleScriptFieldScript.convertFromDouble(v), equalTo(new double[] {v})); + assertThat(DoubleScriptFieldScript.convertFromDef(v), equalTo(new double[] {v})); + } + + public void testConvertFromCollection() { + double d = randomDouble(); + long l = randomLong(); + int i = randomInt(); + assertThat(DoubleScriptFieldScript.convertFromCollection(List.of(d, l, i)), equalTo(new double[] {d, l, i})); + assertThat(DoubleScriptFieldScript.convertFromDef(List.of(d, l, i)), equalTo(new double[] {d, l, i})); + } + + public void testConvertDoubleArrayFromDef() { + double[] a = new double[] {1, 2, 3}; + assertThat(DoubleScriptFieldScript.convertFromDef(a), equalTo(a)); + } } From 0e885f0bc4ea131d05c44e0a99e550fac9da36db Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 25 Aug 2020 10:52:41 -0400 Subject: [PATCH 3/4] Format --- .../runtimefields/DoubleScriptFieldScriptTests.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java index 7bfefc16dbd2f..fcb225e227037 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java @@ -36,20 +36,20 @@ protected DoubleScriptFieldScript.Factory dummyScript() { public void testConvertDouble() { double v = randomDouble(); - assertThat(DoubleScriptFieldScript.convertFromDouble(v), equalTo(new double[] {v})); - assertThat(DoubleScriptFieldScript.convertFromDef(v), equalTo(new double[] {v})); + assertThat(DoubleScriptFieldScript.convertFromDouble(v), equalTo(new double[] { v })); + assertThat(DoubleScriptFieldScript.convertFromDef(v), equalTo(new double[] { v })); } public void testConvertFromCollection() { double d = randomDouble(); long l = randomLong(); int i = randomInt(); - assertThat(DoubleScriptFieldScript.convertFromCollection(List.of(d, l, i)), equalTo(new double[] {d, l, i})); - assertThat(DoubleScriptFieldScript.convertFromDef(List.of(d, l, i)), equalTo(new double[] {d, l, i})); + assertThat(DoubleScriptFieldScript.convertFromCollection(List.of(d, l, i)), equalTo(new double[] { d, l, i })); + assertThat(DoubleScriptFieldScript.convertFromDef(List.of(d, l, i)), equalTo(new double[] { d, l, i })); } public void testConvertDoubleArrayFromDef() { - double[] a = new double[] {1, 2, 3}; + double[] a = new double[] { 1, 2, 3 }; assertThat(DoubleScriptFieldScript.convertFromDef(a), equalTo(a)); } } From 3bb6710b584cc64a23ed842a1d4086f4eb99b2e0 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 25 Aug 2020 13:15:28 -0400 Subject: [PATCH 4/4] Handle all numbers --- .../DoubleScriptFieldScript.java | 4 +-- .../DoubleScriptFieldScriptTests.java | 6 ++++ .../test/runtime_fields/30_double.yml | 30 +++++++++++++++++-- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java index a97175e401648..e732c670e7929 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScript.java @@ -63,8 +63,8 @@ public static double[] convertFromCollection(Collection v) { } public static double[] convertFromDef(Object o) { - if (o instanceof Double) { - return convertFromDouble(((Double) o).doubleValue()); + if (o instanceof Number) { + return convertFromDouble(((Number) o).doubleValue()); } else if (o instanceof Collection) { return convertFromCollection((Collection) o); } else { diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java index fcb225e227037..b33c10b56c100 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/DoubleScriptFieldScriptTests.java @@ -52,4 +52,10 @@ public void testConvertDoubleArrayFromDef() { double[] a = new double[] { 1, 2, 3 }; assertThat(DoubleScriptFieldScript.convertFromDef(a), equalTo(a)); } + + public void testConvertNumberFromDef() { + for (Number n : new Number[] { randomByte(), randomShort(), randomInt(), randomLong(), randomFloat() }) { + assertThat(DoubleScriptFieldScript.convertFromDef(n), equalTo(new double[] { n.doubleValue() })); + } + } } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml index 1d4a596e7aa5d..897330e219c7e 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml @@ -37,7 +37,7 @@ setup: runtime_type: double script: source: | - source['voltage'] / params.max; + source['voltage'] / params.max params: max: 5.8 # Test the implicit conversion from double -> double[] @@ -46,7 +46,22 @@ setup: runtime_type: double script: source: | - ((double) source['voltage']) / 5.8; + ((double) source['voltage']) / 5.8 + # Test the implicit conversion from long -> double[] + voltage_percent_returning_long: + type: runtime_script + runtime_type: double + script: + source: | + (long) (doc['voltage_percent_returning_double'].value * 100) + # Test the implicit conversion from def (secretly a long) -> double[] + voltage_percent_returning_def_long: + type: runtime_script + runtime_type: double + script: + source: | + def v = (long) doc['voltage_percent_returning_long'].value; + return v; # Test fetching many values and the Collection -> double[] conversion voltage_sqrts: type: runtime_script @@ -108,10 +123,19 @@ setup: index: sensor body: sort: timestamp - docvalue_fields: [voltage_percent, voltage_percent_from_source, voltage_percent_returning_double, voltage_sqrts] + docvalue_fields: + - voltage_percent + - voltage_percent_from_source + - voltage_percent_returning_double + - voltage_percent_returning_long + - voltage_percent_returning_def_long + - voltage_sqrts - match: {hits.total.value: 6} - match: {hits.hits.0.fields.voltage_percent: [0.6896551724137931] } - match: {hits.hits.0.fields.voltage_percent_from_source: [0.6896551724137931] } + - match: {hits.hits.0.fields.voltage_percent_returning_double: [0.6896551724137931] } + - match: {hits.hits.0.fields.voltage_percent_returning_long: [68.0] } + - match: {hits.hits.0.fields.voltage_percent_returning_def_long: [68.0] } # Scripts that scripts that emit multiple values are supported and their results are sorted - match: {hits.hits.0.fields.voltage_sqrts: [1.4142135623730951, 2.0, 4.0] } - match: {hits.hits.1.fields.voltage_percent: [0.7241379310344828] }