Skip to content

Commit

Permalink
Convert double script to return array (#61504)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nik9000 authored Aug 25, 2020
1 parent 8818156 commit 6d8170c
Show file tree
Hide file tree
Showing 21 changed files with 175 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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();
}

Expand All @@ -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());"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public AbstractLongScriptFieldScript(Map<String, Object> params, SearchLookup se
super(params, searchLookup, ctx);
}

public abstract void execute();

/**
* Execute the script for the provided {@code docId}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,4 @@ public final Map<String, Object> getSource() {
public final Map<String, ScriptDocValues<?>> getDoc() {
return leafSearchLookup.doc();
}

public abstract void execute();
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public BooleanScriptFieldScript(Map<String, Object> params, SearchLookup searchL
super(params, searchLookup, ctx);
}

public abstract void execute();

/**
* Execute the script for the provided {@code docId}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
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;
import org.elasticsearch.script.ScriptFactory;
import org.elasticsearch.search.lookup.SearchLookup;

import java.io.IOException;
import java.util.Collection;
import java.util.List;
import java.util.Map;

Expand All @@ -35,55 +35,40 @@ public interface LeafFactory {
DoubleScriptFieldScript newInstance(LeafReaderContext ctx) throws IOException;
}

private double[] values = new double[1];
private int count;

public DoubleScriptFieldScript(Map<String, Object> 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 Number) {
return convertFromDouble(((Number) o).doubleValue());
} else if (o instanceof Collection) {
return convertFromCollection((Collection<?>) o);
} else {
return (double[]) o;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ public IpScriptFieldScript(Map<String, Object> params, SearchLookup searchLookup
super(params, searchLookup, ctx);
}

public abstract void execute();

/**
* Execute the script for the provided {@code docId}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public StringScriptFieldScript(Map<String, Object> params, SearchLookup searchLo
super(params, searchLookup, ctx);
}

public abstract void execute();

/**
* Execute the script for the provided {@code docId}.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

public final class ScriptDoubleDocValues extends SortedNumericDoubleValues {
private final DoubleScriptFieldScript script;
private double[] values;
private int cursor;

ScriptDoubleDocValues(DoubleScriptFieldScript script) {
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,19 @@

import org.elasticsearch.script.ScriptContext;

import java.util.List;

import static org.hamcrest.Matchers.equalTo;

public class DoubleScriptFieldScriptTests extends ScriptFieldScriptTestCase<DoubleScriptFieldScript.Factory> {
public static final DoubleScriptFieldScript.Factory DUMMY = (params, lookup) -> ctx -> new DoubleScriptFieldScript(
params,
lookup,
ctx
) {
@Override
public void execute() {
new DoubleScriptFieldScript.Value(this).value(1.0);
public double[] execute() {
return new double[] { 1.0 };
}
};

Expand All @@ -29,4 +33,29 @@ protected ScriptContext<DoubleScriptFieldScript.Factory> 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));
}

public void testConvertNumberFromDef() {
for (Number n : new Number[] { randomByte(), randomShort(), randomInt(), randomLong(), randomFloat() }) {
assertThat(DoubleScriptFieldScript.convertFromDef(n), equalTo(new double[] { n.doubleValue() }));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -270,21 +270,27 @@ private DoubleScriptFieldScript.Factory factory(String code) {
case "read_foo":
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());
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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 6d8170c

Please sign in to comment.