Skip to content

Commit

Permalink
Scripting: Convert script fields to use script context (#34164)
Browse files Browse the repository at this point in the history
This commit removes the use of SearchScript for script fields and adds
a new FieldScript.
  • Loading branch information
rjernst authored Oct 20, 2018
1 parent 7ab4648 commit 222652d
Show file tree
Hide file tree
Showing 21 changed files with 280 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void testGetStoredScript() throws Exception {
Collections.singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType()));

PutStoredScriptRequest request =
new PutStoredScriptRequest(id, "search", new BytesArray("{}"), XContentType.JSON, scriptSource);
new PutStoredScriptRequest(id, "score", new BytesArray("{}"), XContentType.JSON, scriptSource);
assertAcked(execute(request, highLevelClient()::putScript, highLevelClient()::putScriptAsync));

GetStoredScriptRequest getRequest = new GetStoredScriptRequest("calculate-score");
Expand All @@ -66,7 +66,7 @@ public void testDeleteStoredScript() throws Exception {
Collections.singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType()));

PutStoredScriptRequest request =
new PutStoredScriptRequest(id, "search", new BytesArray("{}"), XContentType.JSON, scriptSource);
new PutStoredScriptRequest(id, "score", new BytesArray("{}"), XContentType.JSON, scriptSource);
assertAcked(execute(request, highLevelClient()::putScript, highLevelClient()::putScriptAsync));

DeleteStoredScriptRequest deleteRequest = new DeleteStoredScriptRequest(id);
Expand All @@ -89,7 +89,7 @@ public void testPutScript() throws Exception {
Collections.singletonMap(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType()));

PutStoredScriptRequest request =
new PutStoredScriptRequest(id, "search", new BytesArray("{}"), XContentType.JSON, scriptSource);
new PutStoredScriptRequest(id, "score", new BytesArray("{}"), XContentType.JSON, scriptSource);
assertAcked(execute(request, highLevelClient()::putScript, highLevelClient()::putScriptAsync));

Map<String, Object> script = getAsMap("/_scripts/" + id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public void onFailure(Exception e) {

private void putStoredScript(String id, StoredScriptSource scriptSource) throws IOException {
PutStoredScriptRequest request =
new PutStoredScriptRequest(id, "search", new BytesArray("{}"), XContentType.JSON, scriptSource);
new PutStoredScriptRequest(id, "score", new BytesArray("{}"), XContentType.JSON, scriptSource);
assertAcked(execute(request, highLevelClient()::putScript, highLevelClient()::putScriptAsync));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,15 @@ a customized value for each document in the results of a query.
Contains the fields of the specified document where each field is a
`List` of values.

{ref}/mapping-source-field.html[`ctx['_source']`] (`Map`)::
{ref}/mapping-source-field.html[`params['_source']`] (`Map`, read-only)::
Contains extracted JSON in a `Map` and `List` structure for the fields
existing in a stored document.

`_score` (`double` read-only)::
The original score of the specified document.

*Return*

`Object`::
The customized value for each document.

*API*

The standard <<painless-api-reference, Painless API>> is available.
The standard <<painless-api-reference, Painless API>> is available.
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.script.expression;

import org.apache.lucene.expressions.Expression;
import org.apache.lucene.expressions.SimpleBindings;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.search.DoubleValues;
import org.apache.lucene.search.DoubleValuesSource;
import org.elasticsearch.script.FieldScript;
import org.elasticsearch.script.GeneralScriptException;

import java.io.IOException;

public class ExpressionFieldScript implements FieldScript.LeafFactory {
private final Expression exprScript;
private final DoubleValuesSource source;

ExpressionFieldScript(Expression e, SimpleBindings b) {
this.exprScript = e;
this.source = exprScript.getDoubleValuesSource(b);
}

@Override
public FieldScript newInstance(final LeafReaderContext leaf) throws IOException {
return new FieldScript() {

// Fake the scorer until setScorer is called.
DoubleValues values = source.getValues(leaf, null);

@Override
public Object execute() {
try {
return values.doubleValue();
} catch (Exception exception) {
throw new GeneralScriptException("Error evaluating " + exprScript, exception);
}
}

@Override
public void setDocument(int d) {
try {
values.advanceExact(d);
} catch (IOException e) {
throw new IllegalStateException("Can't advance to doc using " + exprScript, e);
}
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.elasticsearch.script.BucketAggregationScript;
import org.elasticsearch.script.BucketAggregationSelectorScript;
import org.elasticsearch.script.ClassPermission;
import org.elasticsearch.script.FieldScript;
import org.elasticsearch.script.FilterScript;
import org.elasticsearch.script.NumberSortScript;
import org.elasticsearch.script.ScoreScript;
Expand Down Expand Up @@ -139,6 +140,9 @@ public boolean execute() {
} else if (context.instanceClazz.equals(NumberSortScript.class)) {
NumberSortScript.Factory factory = (p, lookup) -> newSortScript(expr, lookup, p);
return context.factoryClazz.cast(factory);
} else if (context.instanceClazz.equals(FieldScript.class)) {
FieldScript.Factory factory = (p, lookup) -> newFieldScript(expr, lookup, p);
return context.factoryClazz.cast(factory);
}
throw new IllegalArgumentException("expression engine does not know how to handle script context [" + context.name + "]");
}
Expand Down Expand Up @@ -289,6 +293,23 @@ private AggregationScript.LeafFactory newAggregationScript(Expression expr, Sear
return new ExpressionAggregationScript(expr, bindings, specialValue);
}

private FieldScript.LeafFactory newFieldScript(Expression expr, SearchLookup lookup, @Nullable Map<String, Object> vars) {
SimpleBindings bindings = new SimpleBindings();
for (String variable : expr.variables) {
try {
if (vars != null && vars.containsKey(variable)) {
bindFromParams(vars, bindings, variable);
} else {
final ValueSource valueSource = getDocValueSource(variable, lookup);
bindings.add(variable, valueSource.asDoubleValuesSource());
}
} catch (Exception e) {
throw convertToScriptException("link error", expr.sourceText, variable, e);
}
}
return new ExpressionFieldScript(expr, bindings);
}

/**
* This is a hack for filter scripts, which must return booleans instead of doubles as expression do.
* See https://github.com/elastic/elasticsearch/issues/26429.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,31 @@
import org.elasticsearch.index.fielddata.IndexNumericFieldData;
import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.NumberFieldMapper.NumberFieldType;
import org.elasticsearch.index.mapper.NumberFieldMapper.NumberType;
import org.elasticsearch.index.mapper.NumberFieldMapper;
import org.elasticsearch.script.FieldScript;
import org.elasticsearch.script.ScriptException;
import org.elasticsearch.script.SearchScript;
import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.test.ESTestCase;

import java.io.IOException;
import java.text.ParseException;
import java.util.Collections;

import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyObject;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class ExpressionTests extends ESTestCase {
public class ExpressionFieldScriptTests extends ESTestCase {
private ExpressionScriptEngine service;
private SearchLookup lookup;

@Override
public void setUp() throws Exception {
super.setUp();

NumberFieldType fieldType = new NumberFieldType(NumberType.DOUBLE);
NumberFieldMapper.NumberFieldType fieldType = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.DOUBLE);
MapperService mapperService = mock(MapperService.class);
when(mapperService.fullName("field")).thenReturn(fieldType);
when(mapperService.fullName("alias")).thenReturn(fieldType);
Expand All @@ -68,18 +68,11 @@ public void setUp() throws Exception {
lookup = new SearchLookup(mapperService, ignored -> fieldData, null);
}

private SearchScript.LeafFactory compile(String expression) {
SearchScript.Factory factory = service.compile(null, expression, SearchScript.CONTEXT, Collections.emptyMap());
private FieldScript.LeafFactory compile(String expression) {
FieldScript.Factory factory = service.compile(null, expression, FieldScript.CONTEXT, Collections.emptyMap());
return factory.newFactory(Collections.emptyMap(), lookup);
}

public void testNeedsScores() {
assertFalse(compile("1.2").needs_score());
assertFalse(compile("doc['field'].value").needs_score());
assertTrue(compile("1/_score").needs_score());
assertTrue(compile("doc['field'].value * _score").needs_score());
}

public void testCompileError() {
ScriptException e = expectThrows(ScriptException.class, () -> {
compile("doc['field'].value * *@#)(@$*@#$ + 4");
Expand All @@ -95,18 +88,18 @@ public void testLinkError() {
}

public void testFieldAccess() throws IOException {
SearchScript script = compile("doc['field'].value").newInstance(null);
FieldScript script = compile("doc['field'].value").newInstance(null);
script.setDocument(1);

double result = script.runAsDouble();
assertEquals(2.718, result, 0.0);
Object result = script.execute();
assertThat(result, equalTo(2.718));
}

public void testFieldAccessWithFieldAlias() throws IOException {
SearchScript script = compile("doc['alias'].value").newInstance(null);
FieldScript script = compile("doc['alias'].value").newInstance(null);
script.setDocument(1);

double result = script.runAsDouble();
assertEquals(2.718, result, 0.0);
Object result = script.execute();
assertThat(result, equalTo(2.718));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void testAllOpsDisabledIndexedScripts() throws IOException {
.setIndices("test").setTypes("scriptTest").get();
fail("search script should have been rejected");
} catch(Exception e) {
assertThat(e.toString(), containsString("cannot execute scripts using [search] context"));
assertThat(e.toString(), containsString("cannot execute scripts using [field] context"));
}
try {
client().prepareSearch("test")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import org.elasticsearch.painless.spi.Whitelist;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptException;
import org.elasticsearch.script.SearchScript;
import org.elasticsearch.test.ESTestCase;
import org.junit.Before;

Expand Down Expand Up @@ -67,7 +66,6 @@ protected Settings scriptEngineSettings() {
*/
protected Map<ScriptContext<?>, List<Whitelist>> scriptContexts() {
Map<ScriptContext<?>, List<Whitelist>> contexts = new HashMap<>();
contexts.put(SearchScript.CONTEXT, Whitelist.BASE_WHITELISTS);
contexts.put(PainlessTestScript.CONTEXT, Whitelist.BASE_WHITELISTS);
return contexts;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@
catch: bad_request
put_script:
id: "1"
context: "search"
context: "score"
body: { "script": {"lang": "painless", "source": "_score * foo bar + doc['myParent.weight'].value"} }

- do:
catch: /compile error/
put_script:
id: "1"
context: "search"
context: "score"
body: { "script": {"lang": "painless", "source": "_score * foo bar + doc['myParent.weight'].value"} }
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@

package org.elasticsearch.example.painlesswhitelist;

import java.util.Collections;
import java.util.List;
import java.util.Map;

import org.elasticsearch.painless.spi.PainlessExtension;
import org.elasticsearch.painless.spi.Whitelist;
import org.elasticsearch.painless.spi.WhitelistLoader;
import org.elasticsearch.script.FieldScript;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.SearchScript;

import java.util.Collections;
import java.util.List;
import java.util.Map;

/** An extension of painless which adds a whitelist. */
public class ExampleWhitelistExtension implements PainlessExtension {
Expand All @@ -37,6 +37,6 @@ public class ExampleWhitelistExtension implements PainlessExtension {

@Override
public Map<ScriptContext<?>, List<Whitelist>> getContextWhitelists() {
return Collections.singletonMap(SearchScript.CONTEXT, Collections.singletonList(WHITELIST));
return Collections.singletonMap(FieldScript.CONTEXT, Collections.singletonList(WHITELIST));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
package org.elasticsearch.index.query;

import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.script.SearchScript;
import org.elasticsearch.script.FieldScript;
import org.elasticsearch.search.builder.SearchSourceBuilder;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
import org.elasticsearch.search.fetch.subphase.InnerHitsContext;
Expand Down Expand Up @@ -88,10 +88,10 @@ protected void setupInnerHitsContext(QueryShardContext queryShardContext,
if (innerHitBuilder.getScriptFields() != null) {
for (SearchSourceBuilder.ScriptField field : innerHitBuilder.getScriptFields()) {
QueryShardContext innerContext = innerHitsContext.getQueryShardContext();
SearchScript.Factory factory = innerContext.getScriptService().compile(field.script(), SearchScript.CONTEXT);
SearchScript.LeafFactory searchScript = factory.newFactory(field.script().getParams(), innerHitsContext.lookup());
FieldScript.Factory factory = innerContext.getScriptService().compile(field.script(), FieldScript.CONTEXT);
FieldScript.LeafFactory fieldScript = factory.newFactory(field.script().getParams(), innerHitsContext.lookup());
innerHitsContext.scriptFields().add(new org.elasticsearch.search.fetch.subphase.ScriptFieldsContext.ScriptField(
field.fieldName(), searchScript, field.ignoreFailure()));
field.fieldName(), fieldScript, field.ignoreFailure()));
}
}
if (innerHitBuilder.getFetchSourceContext() != null) {
Expand Down
Loading

0 comments on commit 222652d

Please sign in to comment.