From a7f538c0d359890459a38f0e19fdb734af3e8023 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 24 Feb 2021 12:26:51 +0000 Subject: [PATCH 01/39] Take 3... Just re-use scripts, exposing things via a MemoryIndex if necessary --- .../index/mapper/DocumentParser.java | 2 + .../index/mapper/FieldAliasMapper.java | 5 + .../index/mapper/FieldMapper.java | 12 ++ .../index/mapper/IndexTimeScriptContext.java | 189 ++++++++++++++++++ .../elasticsearch/index/mapper/Mapper.java | 6 + .../index/mapper/NumberFieldMapper.java | 119 ++++++++++- .../index/mapper/ObjectMapper.java | 7 + .../index/mapper/ParseContext.java | 25 +++ .../index/mapper/ScriptParams.java | 76 +++++++ .../index/mapper/CalculatedFieldTests.java | 151 ++++++++++++++ 10 files changed, 591 insertions(+), 1 deletion(-) create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java create mode 100644 server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index f613003967a87..a0d87e93a2eab 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -106,6 +106,8 @@ private static void internalParseDocument(RootObjectMapper root, MetadataFieldMa parseObjectOrNested(context, root); } + root.postParse(context); + for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) { metadataMapper.postParse(context); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java index ad5163cda58ed..514c2cc789cbf 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java @@ -108,6 +108,11 @@ public void validate(MappingLookup mappers) { } } + @Override + public void postParse(ParseContext context) throws IOException { + + } + public static class TypeParser implements Mapper.TypeParser { @Override public Mapper.Builder parse(String name, Map node, ParserContext parserContext) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index 8851a442f9946..e4e2f0d598bdc 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -167,6 +167,18 @@ public void parse(ParseContext context) throws IOException { multiFields.parse(this, context); } + @Override + public void postParse(ParseContext context) throws IOException { // TODO make final, convert metadatamappers to doPostParse + doPostParse(context); + for (Mapper mapper : this) { + mapper.postParse(context); + } + } + + protected void doPostParse(ParseContext context) throws IOException { + // no-op by default + } + /** * Parse the field value and populate the fields on {@link ParseContext#doc()}. * diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java b/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java new file mode 100644 index 0000000000000..e78310dfca010 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java @@ -0,0 +1,189 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.index.mapper; + +import org.apache.lucene.index.BinaryDocValues; +import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.index.FieldInfo; +import org.apache.lucene.index.FieldInfos; +import org.apache.lucene.index.Fields; +import org.apache.lucene.index.IndexOptions; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.index.LeafMetaData; +import org.apache.lucene.index.LeafReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.PointValues; +import org.apache.lucene.index.SortedDocValues; +import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.index.SortedSetDocValues; +import org.apache.lucene.index.StoredFieldVisitor; +import org.apache.lucene.index.Terms; +import org.apache.lucene.index.memory.MemoryIndex; +import org.apache.lucene.util.Bits; +import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.search.lookup.SearchLookup; + +import java.io.IOException; +import java.util.Collections; + +public class IndexTimeScriptContext { + + public final SearchLookup searchLookup; + public final LeafReaderContext leafReaderContext; + + public IndexTimeScriptContext(SearchLookup searchLookup, ParseContext.Document doc, BytesReference sourceBytes) { + this.searchLookup = searchLookup; + LazyDocumentReader reader = new LazyDocumentReader(doc, sourceBytes); + this.leafReaderContext = reader.getContext(); + } + + private static class LazyDocumentReader extends LeafReader { + + private final ParseContext.Document document; + private final BytesReference sourceBytes; + private LeafReaderContext in; + + private LazyDocumentReader(ParseContext.Document document, BytesReference sourceBytes) { + this.document = document; + this.sourceBytes = sourceBytes; + } + + private void buildDocValues() { + if (in != null) { + return; + } + MemoryIndex mi = new MemoryIndex(); + for (IndexableField f : document) { + if (f.fieldType().docValuesType() != null) { + mi.addField(f, null); + } + } + mi.freeze(); + this.in = mi.createSearcher().getIndexReader().leaves().get(0); + } + + @Override + public NumericDocValues getNumericDocValues(String field) throws IOException { + buildDocValues(); + return in.reader().getNumericDocValues(field); + } + + @Override + public BinaryDocValues getBinaryDocValues(String field) throws IOException { + buildDocValues(); + return in.reader().getBinaryDocValues(field); + } + + @Override + public SortedDocValues getSortedDocValues(String field) throws IOException { + buildDocValues(); + return in.reader().getSortedDocValues(field); + } + + @Override + public SortedNumericDocValues getSortedNumericDocValues(String field) throws IOException { + buildDocValues(); + return in.reader().getSortedNumericDocValues(field); + } + + @Override + public SortedSetDocValues getSortedSetDocValues(String field) throws IOException { + buildDocValues(); + return in.reader().getSortedSetDocValues(field); + } + + @Override + public FieldInfos getFieldInfos() { + buildDocValues(); + return in.reader().getFieldInfos(); + } + + @Override + public void document(int docID, StoredFieldVisitor visitor) throws IOException { + visitor.binaryField(SOURCE_FIELD_INFO, sourceBytes.toBytesRef().bytes); + } + + @Override + public CacheHelper getCoreCacheHelper() { + throw new UnsupportedOperationException(); + } + + @Override + public Terms terms(String field) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public NumericDocValues getNormValues(String field) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public Bits getLiveDocs() { + throw new UnsupportedOperationException(); + } + + @Override + public PointValues getPointValues(String field) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public void checkIntegrity() throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public LeafMetaData getMetaData() { + throw new UnsupportedOperationException(); + } + + @Override + public Fields getTermVectors(int docID) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public int numDocs() { + throw new UnsupportedOperationException(); + } + + @Override + public int maxDoc() { + throw new UnsupportedOperationException(); + } + + @Override + protected void doClose() throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public CacheHelper getReaderCacheHelper() { + throw new UnsupportedOperationException(); + } + } + + private static final FieldInfo SOURCE_FIELD_INFO = new FieldInfo( + "_source", + 0, + false, + false, + false, + IndexOptions.NONE, + DocValuesType.NONE, + -1, + Collections.emptyMap(), + 0, + 0, + 0, + false + ); +} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java b/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java index e8c073562ce1e..409b2655072e2 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java @@ -18,6 +18,7 @@ import org.elasticsearch.index.similarity.SimilarityProvider; import org.elasticsearch.script.ScriptService; +import java.io.IOException; import java.util.Map; import java.util.Objects; import java.util.function.BooleanSupplier; @@ -205,4 +206,9 @@ public final String simpleName() { */ public abstract void validate(MappingLookup mappers); + /** + * Perform mapping actions after a document's source has been parsed + */ + public abstract void postParse(ParseContext context) throws IOException; + } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 3b0a77f25c000..c5587a4b8953c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -36,6 +36,10 @@ import org.elasticsearch.index.fielddata.IndexNumericFieldData.NumericType; import org.elasticsearch.index.fielddata.plain.SortedNumericIndexFieldData; import org.elasticsearch.index.query.SearchExecutionContext; +import org.elasticsearch.runtimefields.mapper.DoubleFieldScript; +import org.elasticsearch.runtimefields.mapper.LongFieldScript; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.lookup.SearchLookup; @@ -48,6 +52,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.function.BiConsumer; import java.util.function.BiFunction; import java.util.function.Function; import java.util.function.Supplier; @@ -73,6 +78,8 @@ public static class Builder extends FieldMapper.Builder { private final Parameter nullValue; + private final Parameter> script; + private final Parameter> meta = Parameter.metaParam(); private final NumberType type; @@ -96,6 +103,10 @@ public Builder(String name, NumberType type, boolean ignoreMalformedByDefault, b = Parameter.explicitBoolParam("coerce", true, m -> toType(m).coerce, coerceByDefault); this.nullValue = new Parameter<>("null_value", false, () -> null, (n, c, o) -> o == null ? null : type.parse(o, false), m -> toType(m).nullValue).acceptsNull(); + this.script = ScriptParams.script( + type.compiler(name), + m -> toType(m).script + ); } Builder nullValue(Number number) { @@ -110,7 +121,7 @@ public Builder docValues(boolean hasDocValues) { @Override protected List> getParameters() { - return List.of(indexed, hasDocValues, stored, ignoreMalformed, coerce, nullValue, meta); + return List.of(indexed, hasDocValues, stored, ignoreMalformed, coerce, nullValue, script, meta); } @Override @@ -138,6 +149,11 @@ public Float parse(Object value, boolean coerce) { return result; } + @Override + public BiFunction compiler(String fieldName) { + return DOUBLE.compiler(fieldName); + } + @Override public Number parsePoint(byte[] value) { return HalfFloatPoint.decodeDimension(value, 0); @@ -248,6 +264,11 @@ public Float parse(XContentParser parser, boolean coerce) throws IOException { return parsed; } + @Override + public BiFunction compiler(String fieldName) { + return DOUBLE.compiler(fieldName); + } + @Override public Query termQuery(String field, Object value) { float v = parse(value, false); @@ -335,6 +356,12 @@ public Double parse(XContentParser parser, boolean coerce) throws IOException { return parsed; } + @Override + public BiFunction compiler(String fieldName) { + return (script, service) + -> new DoubleMapperScript(fieldName, service.compile(script, DoubleFieldScript.CONTEXT), script.getParams()); + } + @Override public Query termQuery(String field, Object value) { double v = parse(value, false); @@ -410,6 +437,11 @@ public Number parsePoint(byte[] value) { return INTEGER.parsePoint(value).byteValue(); } + @Override + public BiFunction compiler(String fieldName) { + return LONG.compiler(fieldName); + } + @Override public Short parse(XContentParser parser, boolean coerce) throws IOException { int value = parser.intValue(coerce); @@ -476,6 +508,11 @@ public Short parse(XContentParser parser, boolean coerce) throws IOException { return parser.shortValue(coerce); } + @Override + public BiFunction compiler(String fieldName) { + return LONG.compiler(fieldName); + } + @Override public Query termQuery(String field, Object value) { return INTEGER.termQuery(field, value); @@ -533,6 +570,11 @@ public Integer parse(XContentParser parser, boolean coerce) throws IOException { return parser.intValue(coerce); } + @Override + public BiFunction compiler(String fieldName) { + return LONG.compiler(fieldName); + } + @Override public Query termQuery(String field, Object value) { if (hasDecimalPart(value)) { @@ -638,6 +680,12 @@ public Long parse(XContentParser parser, boolean coerce) throws IOException { return parser.longValue(coerce); } + @Override + public BiFunction compiler(String fieldName) { + return (script, service) + -> new LongMapperScript(fieldName, service.compile(script, LongFieldScript.CONTEXT), script.getParams()); + } + @Override public Query termQuery(String field, Object value) { if (hasDecimalPart(value)) { @@ -722,6 +770,7 @@ public final NumericType numericType() { public final TypeParser parser() { return parser; } + public abstract BiFunction compiler(String fieldName); public abstract Query termQuery(String field, Object value); public abstract Query termsQuery(String field, Collection values); public abstract Query rangeQuery(String field, Object lowerTerm, Object upperTerm, @@ -1001,6 +1050,7 @@ public CollapseType collapseType() { private final Explicit ignoreMalformed; private final Explicit coerce; private final Number nullValue; + private final ScriptParams.CompiledScriptParameter script; private final boolean ignoreMalformedByDefault; private final boolean coerceByDefault; @@ -1021,6 +1071,7 @@ private NumberFieldMapper( this.nullValue = builder.nullValue.getValue(); this.ignoreMalformedByDefault = builder.ignoreMalformed.getDefaultValue().value(); this.coerceByDefault = builder.coerce.getDefaultValue().value(); + this.script = builder.script.get(); } boolean coerce() { @@ -1080,6 +1131,10 @@ protected void parseCreateField(ParseContext context) throws IOException { numericValue = fieldType().type.parse(value, coerce.value()); } + indexValue(context, numericValue); + } + + private void indexValue(ParseContext context, Number numericValue) { context.doc().addAll(fieldType().type.createFields(fieldType().name(), numericValue, indexed, hasDocValues, stored)); @@ -1088,8 +1143,70 @@ protected void parseCreateField(ParseContext context) throws IOException { } } + @Override + protected void doPostParse(ParseContext context) { + if (this.script == null) { + return; + } + this.script.compiledScript.executeAndIndex(context, this::indexValue); + } + @Override public FieldMapper.Builder getMergeBuilder() { return new Builder(simpleName(), type, ignoreMalformedByDefault, coerceByDefault).init(this); } + + private interface MapperScript { + void executeAndIndex(ParseContext context, BiConsumer emitter); + } + + private static class LongMapperScript implements MapperScript { + + final LongFieldScript.Factory scriptFactory; + final Map scriptParams; + final String fieldName; + + private LongMapperScript(String fieldName, LongFieldScript.Factory scriptFactory, Map scriptParams) { + this.scriptFactory = scriptFactory; + this.scriptParams = scriptParams; + this.fieldName = fieldName; + } + + @Override + public void executeAndIndex(ParseContext context, BiConsumer emitter) { + IndexTimeScriptContext scriptContext = context.getScriptContext(); + LongFieldScript s = scriptFactory + .newFactory(fieldName, scriptParams, scriptContext.searchLookup) + .newInstance(scriptContext.leafReaderContext); + s.runForDoc(0); + for (long v : s.values()) { + emitter.accept(context, v); + } + } + } + + private static class DoubleMapperScript implements MapperScript { + + final DoubleFieldScript.Factory scriptFactory; + final Map scriptParams; + final String fieldName; + + private DoubleMapperScript(String fieldName, DoubleFieldScript.Factory scriptFactory, Map scriptParams) { + this.scriptFactory = scriptFactory; + this.scriptParams = scriptParams; + this.fieldName = fieldName; + } + + @Override + public void executeAndIndex(ParseContext context, BiConsumer emitter) { + IndexTimeScriptContext scriptContext = context.getScriptContext(); + DoubleFieldScript s = scriptFactory + .newFactory(fieldName, scriptParams, scriptContext.searchLookup) + .newInstance(scriptContext.leafReaderContext); + s.runForDoc(0); + for (double v : s.values()) { + emitter.accept(context, v); + } + } + } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index 0b802eaba9486..844ebfa57646d 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -569,4 +569,11 @@ void toXContent(XContentBuilder builder, Params params, ToXContent custom) throw protected void doXContent(XContentBuilder builder, Params params) throws IOException { } + + @Override + public void postParse(ParseContext context) throws IOException { + for (Mapper mapper : this) { + mapper.postParse(context); + } + } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java b/server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java index 3d1eeb48d68ed..dc6eddaa52892 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java @@ -15,6 +15,9 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.analysis.IndexAnalyzers; +import org.elasticsearch.index.fielddata.IndexFieldDataCache; +import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; +import org.elasticsearch.search.lookup.SearchLookup; import java.util.ArrayList; import java.util.Collection; @@ -168,6 +171,11 @@ public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter return in.parserContext(dateFormatter); } + @Override + public IndexTimeScriptContext getScriptContext() { + return in.getScriptContext(); + } + @Override public boolean isWithinCopyTo() { return in.isWithinCopyTo(); @@ -311,6 +319,7 @@ public static class InternalParseContext extends ParseContext { private SeqNoFieldMapper.SequenceIDFields seqID; private long numNestedDocs; private boolean docsReversed = false; + private IndexTimeScriptContext scriptContext = null; public InternalParseContext(MappingLookup mappingLookup, Function parserContextFunction, @@ -332,6 +341,20 @@ public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter return parserContextFunction.apply(dateFormatter); } + @Override + public IndexTimeScriptContext getScriptContext() { + if (scriptContext == null) { + SearchLookup lookup = new SearchLookup( + this.mappingLookup::getFieldType, + (ft, s) -> ft.fielddataBuilder("", s).build( + new IndexFieldDataCache.None(), + new NoneCircuitBreakerService()) + ); + scriptContext = new IndexTimeScriptContext(lookup, document, this.sourceToParse.source()); + } + return scriptContext; + } + @Override public IndexSettings indexSettings() { return this.mappingLookup.getIndexSettings(); @@ -511,6 +534,8 @@ public Collection getIgnoredFields() { public abstract Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter); + public abstract IndexTimeScriptContext getScriptContext(); + /** * Return a new context that will be within a copy-to operation. */ diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java b/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java new file mode 100644 index 0000000000000..76d49115e2c04 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java @@ -0,0 +1,76 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.index.mapper; + +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService; + +import java.io.IOException; +import java.util.Objects; +import java.util.function.BiFunction; +import java.util.function.Function; + +public final class ScriptParams { + + private ScriptParams() { } + + public static class CompiledScriptParameter implements ToXContent { + final Script script; + final T compiledScript; + + public CompiledScriptParameter(Script script, T compiledScript) { + this.script = script; + this.compiledScript = compiledScript; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + CompiledScriptParameter that = (CompiledScriptParameter) o; + return Objects.equals(script, that.script); + } + + @Override + public int hashCode() { + return Objects.hash(script); + } + + @Override + public String toString() { + return script.toString(); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return script.toXContent(builder, params); + } + } + + public static FieldMapper.Parameter> script( + BiFunction compiler, + Function>initializer + ) { + return new FieldMapper.Parameter<>( + "script", + false, + () -> null, + (n, c, o) -> { + if (o == null) { + return null; + } + Script script = Script.parse(o); + return new CompiledScriptParameter<>(script, compiler.apply(script, c.scriptService())); + }, + initializer + ).acceptsNull(); + } +} diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java new file mode 100644 index 0000000000000..6b4d380e36429 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -0,0 +1,151 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.index.mapper; + +import org.apache.lucene.index.IndexableField; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.ScriptPlugin; +import org.elasticsearch.runtimefields.mapper.DoubleFieldScript; +import org.elasticsearch.runtimefields.mapper.LongFieldScript; +import org.elasticsearch.script.ScriptContext; +import org.elasticsearch.script.ScriptEngine; + +import java.io.IOException; +import java.util.Collection; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +public class CalculatedFieldTests extends MapperServiceTestCase { + + @Override + protected Collection getPlugins() { + return Set.of(new TestScriptPlugin()); + } + + public void testCalculatedFieldLength() throws IOException { + DocumentMapper mapper = createDocumentMapper(mapping(b -> { + b.startObject("message").field("type", "text").endObject(); + b.startObject("message_length"); + b.field("type", "long"); + b.field("script", "length"); + b.endObject(); + })); + + ParsedDocument doc = mapper.parse(source(b -> b.field("message", "this is some text"))); + IndexableField[] lengthFields = doc.rootDoc().getFields("message_length"); + assertEquals(2, lengthFields.length); + assertEquals("LongPoint ", lengthFields[0].toString()); + assertEquals("docValuesType=SORTED_NUMERIC", lengthFields[1].toString()); + } + + public void testDocAccess() throws IOException { + DocumentMapper mapper = createDocumentMapper(mapping(b -> { + b.startObject("long_field").field("type", "long").endObject(); + b.startObject("long_field_plus_two"); + b.field("type", "long"); + b.field("script", "plus_two"); + b.endObject(); + })); + + ParsedDocument doc = mapper.parse(source(b -> b.field("long_field", 4))); + assertEquals(doc.rootDoc().getField("long_field_plus_two").numericValue(), 6L); + } + + public void testDoublesAccess() throws IOException { + DocumentMapper mapper = createDocumentMapper(mapping(b -> { + b.startObject("double_field").field("type", "double").endObject(); + b.startObject("double_field_plus_two"); + b.field("type", "double"); + b.field("script", "plus_two"); + b.endObject(); + })); + + ParsedDocument doc = mapper.parse(source(b -> b.field("double_field", 4.5))); + assertEquals(doc.rootDoc().getField("double_field_plus_two").numericValue(), 6.5); + } + + public void testSerialization() throws IOException { + DocumentMapper mapper = createDocumentMapper(mapping(b -> { + b.startObject("message").field("type", "text").endObject(); + b.startObject("message_length"); + b.field("type", "long"); + b.field("script", "length"); + b.endObject(); + })); + assertEquals( + "{\"_doc\":{\"properties\":{\"message\":{\"type\":\"text\"}," + + "\"message_length\":{\"type\":\"long\",\"script\":{\"source\":\"length\",\"lang\":\"painless\"}}}}}", + Strings.toString(mapper.mapping())); + } + + public static class TestScriptPlugin extends Plugin implements ScriptPlugin { + + @Override + public ScriptEngine getScriptEngine(Settings settings, Collection> contexts) { + return new ScriptEngine() { + @Override + public String getType() { + return "painless"; + } + + @Override + @SuppressWarnings("unchecked") + public FactoryType compile( + String name, + String code, + ScriptContext context, + Map params + ) { + if (context.factoryClazz == LongFieldScript.Factory.class) { + switch (code) { + case "length": + return (FactoryType) (LongFieldScript.Factory) (n, p, lookup) -> ctx -> new LongFieldScript(n, p, lookup, ctx) { + @Override + public void execute() { + for (Object v : extractFromSource("message")) { + emit(Objects.toString(v).length()); + } + } + }; + case "plus_two": + return (FactoryType) (LongFieldScript.Factory) (n, p, lookup) -> ctx -> new LongFieldScript(n, p, lookup, ctx) { + @Override + public void execute() { + long input = (long) getDoc().get("long_field").get(0); + emit(input + 2); + } + }; + } + } + if (context.factoryClazz == DoubleFieldScript.Factory.class) { + if (code.equals("plus_two")) { + return (FactoryType) (DoubleFieldScript.Factory) (n, p, lookup) -> ctx -> new DoubleFieldScript(n, p, lookup, ctx) { + @Override + public void execute() { + double input = (double) getDoc().get("double_field").get(0); + emit(input + 2); + } + }; + } + } + throw new IllegalArgumentException("Unknown factory type " + context.factoryClazz + " for code " + code); + } + + @Override + public Set> getSupportedContexts() { + return Set.of(LongFieldScript.CONTEXT, DoubleFieldScript.CONTEXT); + } + }; + } + } + +} From 1a330f6b4ab27e79756abb66a06985755457aaa7 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 24 Feb 2021 15:56:24 +0000 Subject: [PATCH 02/39] add test for cross references --- .../index/mapper/IndexTimeScriptContext.java | 10 +++++- .../index/mapper/CalculatedFieldTests.java | 31 +++++++++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java b/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java index e78310dfca010..e81b5e740d9ab 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java @@ -8,6 +8,7 @@ package org.elasticsearch.index.mapper; +import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.index.BinaryDocValues; import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.FieldInfo; @@ -62,7 +63,7 @@ private void buildDocValues() { MemoryIndex mi = new MemoryIndex(); for (IndexableField f : document) { if (f.fieldType().docValuesType() != null) { - mi.addField(f, null); + mi.addField(f, EMPTY_ANALYZER); } } mi.freeze(); @@ -186,4 +187,11 @@ public CacheHelper getReaderCacheHelper() { 0, false ); + + private static final Analyzer EMPTY_ANALYZER = new Analyzer() { + @Override + protected TokenStreamComponents createComponents(String fieldName) { + return new TokenStreamComponents(reader -> {}, null); + } + }; } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index 6b4d380e36429..2e644ad0ea3c5 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -87,6 +87,23 @@ public void testSerialization() throws IOException { Strings.toString(mapper.mapping())); } + public void testCrossReferences() throws IOException { + DocumentMapper mapper = createDocumentMapper(mapping(b -> { + b.startObject("message").field("type", "text").endObject(); + b.startObject("message_length_plus_two"); + b.field("type", "long"); + b.field("script", "message_length_plus_two"); + b.endObject(); + b.startObject("message_length"); + b.field("type", "long"); + b.field("script", "length"); + b.endObject(); + })); + ParsedDocument doc = mapper.parse(source(b -> b.field("message", "this is a message"))); + assertEquals(doc.rootDoc().getField("message_length_plus_two").numericValue(), 19L); + assertEquals(doc.rootDoc().getField("message_length").numericValue(), 17L); + } + public static class TestScriptPlugin extends Plugin implements ScriptPlugin { @Override @@ -108,7 +125,7 @@ public FactoryType compile( if (context.factoryClazz == LongFieldScript.Factory.class) { switch (code) { case "length": - return (FactoryType) (LongFieldScript.Factory) (n, p, lookup) -> ctx -> new LongFieldScript(n, p, lookup, ctx) { + return (FactoryType) (LongFieldScript.Factory) (n, p, l) -> ctx -> new LongFieldScript(n, p, l, ctx) { @Override public void execute() { for (Object v : extractFromSource("message")) { @@ -117,18 +134,26 @@ public void execute() { } }; case "plus_two": - return (FactoryType) (LongFieldScript.Factory) (n, p, lookup) -> ctx -> new LongFieldScript(n, p, lookup, ctx) { + return (FactoryType) (LongFieldScript.Factory) (n, p, l) -> ctx -> new LongFieldScript(n, p, l, ctx) { @Override public void execute() { long input = (long) getDoc().get("long_field").get(0); emit(input + 2); } }; + case "message_length_plus_two": + return (FactoryType) (LongFieldScript.Factory) (n, p, l) -> ctx -> new LongFieldScript(n, p, l, ctx) { + @Override + public void execute() { + long input = (long) getDoc().get("message_length").get(0); + emit(input + 2); + } + }; } } if (context.factoryClazz == DoubleFieldScript.Factory.class) { if (code.equals("plus_two")) { - return (FactoryType) (DoubleFieldScript.Factory) (n, p, lookup) -> ctx -> new DoubleFieldScript(n, p, lookup, ctx) { + return (FactoryType) (DoubleFieldScript.Factory) (n, p, l) -> ctx -> new DoubleFieldScript(n, p, l, ctx) { @Override public void execute() { double input = (double) getDoc().get("double_field").get(0); From 5e1cf0307bc43b38c6910f6dd66cea55e7ad784a Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 4 Mar 2021 15:23:49 +0000 Subject: [PATCH 03/39] Only support script on long and double --- .../index/mapper/NumberFieldMapper.java | 20 ++++++++++++++----- .../index/mapper/NumberFieldMapperTests.java | 17 ++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index c5587a4b8953c..3e0f9b82efe6e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -151,7 +151,9 @@ public Float parse(Object value, boolean coerce) { @Override public BiFunction compiler(String fieldName) { - return DOUBLE.compiler(fieldName); + return (s, ss) -> { + throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); + }; } @Override @@ -266,7 +268,9 @@ public Float parse(XContentParser parser, boolean coerce) throws IOException { @Override public BiFunction compiler(String fieldName) { - return DOUBLE.compiler(fieldName); + return (s, ss) -> { + throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); + }; } @Override @@ -439,7 +443,9 @@ public Number parsePoint(byte[] value) { @Override public BiFunction compiler(String fieldName) { - return LONG.compiler(fieldName); + return (s, ss) -> { + throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); + }; } @Override @@ -510,7 +516,9 @@ public Short parse(XContentParser parser, boolean coerce) throws IOException { @Override public BiFunction compiler(String fieldName) { - return LONG.compiler(fieldName); + return (s, ss) -> { + throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); + }; } @Override @@ -572,7 +580,9 @@ public Integer parse(XContentParser parser, boolean coerce) throws IOException { @Override public BiFunction compiler(String fieldName) { - return LONG.compiler(fieldName); + return (s, ss) -> { + throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); + }; } @Override diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java index db1fd72cc882c..3fde13ffcc864 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -287,4 +287,21 @@ public void testLongIndexingOutOfRange() throws Exception { ); assertEquals(0, doc.rootDoc().getFields("field").length); } + + public void testScriptableTypes() { + Set scriptableTypes = Set.of("long", "double"); + for (String type : types()) { + if (scriptableTypes.contains(type)) { + // won't actually compile because we don't have painless in unit tests, but we can + // check that it gets as far as trying to compile it + Exception e = expectThrows(MapperParsingException.class, + () -> createDocumentMapper(fieldMapping(b -> b.field("type", type).field("script", "foo")))); + assertEquals("Failed to parse mapping: script_lang not supported [painless]", e.getMessage()); + } else { + Exception e = expectThrows(MapperParsingException.class, "Missing exception on type " + type, + () -> createDocumentMapper(fieldMapping(b -> b.field("type", type).field("script", "foo")))); + assertEquals("Failed to parse mapping: Unknown parameter [script] for mapper [field]", e.getMessage()); + } + } + } } From 6d77d28d16e9d9f00efdfaa550171dfca4ba4441 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 5 Mar 2021 12:25:50 +0000 Subject: [PATCH 04/39] Allow calculated fields to refer to each other --- .../index/mapper/DocumentParser.java | 8 +- .../index/mapper/FieldAliasMapper.java | 5 -- .../index/mapper/FieldMapper.java | 16 ++-- .../index/mapper/IndexTimeScriptContext.java | 83 ++++++++++++++++--- .../elasticsearch/index/mapper/Mapper.java | 6 -- .../index/mapper/MappingLookup.java | 11 +++ .../index/mapper/NumberFieldMapper.java | 45 +++++----- .../index/mapper/ObjectMapper.java | 6 -- .../index/mapper/ParseContext.java | 25 ------ .../index/mapper/CalculatedFieldTests.java | 12 ++- 10 files changed, 124 insertions(+), 93 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 820b2defccb90..6832370564d88 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -61,7 +61,7 @@ ParsedDocument parseDocument(SourceToParse source, source, parser); validateStart(parser); - internalParseDocument(mappingLookup.getMapping().getRoot(), metadataFieldsMappers, context, parser); + internalParseDocument(mappingLookup, metadataFieldsMappers, context, parser); validateEnd(parser); } catch (Exception e) { throw wrapInMapperParsingException(source, e); @@ -91,8 +91,10 @@ private static boolean containsDisabledObjectMapper(ObjectMapper objectMapper, S return false; } - private static void internalParseDocument(RootObjectMapper root, MetadataFieldMapper[] metadataFieldsMappers, + private static void internalParseDocument(MappingLookup lookup, MetadataFieldMapper[] metadataFieldsMappers, ParseContext context, XContentParser parser) throws IOException { + + RootObjectMapper root = lookup.getMapping().getRoot(); final boolean emptyDoc = isEmptyDoc(root, parser); for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) { @@ -106,7 +108,7 @@ private static void internalParseDocument(RootObjectMapper root, MetadataFieldMa parseObjectOrNested(context, root); } - root.postParse(context); + IndexTimeScriptContext.executePostParsePhases(lookup, context); for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) { metadataMapper.postParse(context); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java index 514c2cc789cbf..ad5163cda58ed 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldAliasMapper.java @@ -108,11 +108,6 @@ public void validate(MappingLookup mappers) { } } - @Override - public void postParse(ParseContext context) throws IOException { - - } - public static class TypeParser implements Mapper.TypeParser { @Override public Mapper.Builder parse(String name, Map node, ParserContext parserContext) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index e4e2f0d598bdc..1e9506c69b5e1 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -10,6 +10,7 @@ import org.apache.lucene.document.Field; import org.elasticsearch.Version; +import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.TriFunction; import org.elasticsearch.common.logging.DeprecationCategory; @@ -167,16 +168,11 @@ public void parse(ParseContext context) throws IOException { multiFields.parse(this, context); } - @Override - public void postParse(ParseContext context) throws IOException { // TODO make final, convert metadatamappers to doPostParse - doPostParse(context); - for (Mapper mapper : this) { - mapper.postParse(context); - } - } - - protected void doPostParse(ParseContext context) throws IOException { - // no-op by default + /** + * Returns a post-parse executor for this field mapper, or {@code null} if none is defined + */ + public CheckedConsumer getPostParsePhase() { + return null; } /** diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java b/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java index e81b5e740d9ab..07f56501d5e74 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java @@ -28,35 +28,90 @@ import org.apache.lucene.index.Terms; import org.apache.lucene.index.memory.MemoryIndex; import org.apache.lucene.util.Bits; +import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.index.fielddata.IndexFieldDataCache; +import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; public class IndexTimeScriptContext { public final SearchLookup searchLookup; public final LeafReaderContext leafReaderContext; + public final ParseContext pc; + private final Map mapperScripts = new HashMap<>(); - public IndexTimeScriptContext(SearchLookup searchLookup, ParseContext.Document doc, BytesReference sourceBytes) { - this.searchLookup = searchLookup; - LazyDocumentReader reader = new LazyDocumentReader(doc, sourceBytes); + public static void executePostParsePhases(MappingLookup lookup, ParseContext parseContext) throws IOException { + if (lookup.getPostParsePhases().isEmpty()) { + return; + } + IndexTimeScriptContext context = new IndexTimeScriptContext(lookup, parseContext); + context.executePostParse(); + } + + private IndexTimeScriptContext(MappingLookup mappingLookup, ParseContext pc) { + this.searchLookup = new SearchLookup( + mappingLookup::getFieldType, + (ft, s) -> ft.fielddataBuilder("", s).build( + new IndexFieldDataCache.None(), + new NoneCircuitBreakerService()) + ); + this.pc = pc; + LazyDocumentReader reader = new LazyDocumentReader( + pc.rootDoc(), + pc.sourceToParse().source(), + mappingLookup.getPostParsePhases().keySet()); this.leafReaderContext = reader.getContext(); + mappingLookup.getPostParsePhases().forEach((k, c) -> mapperScripts.put(k, new MapperScript(c))); } - private static class LazyDocumentReader extends LeafReader { + private void executePostParse() throws IOException { + for (MapperScript ms : mapperScripts.values()) { + ms.script.accept(this); + } + } + + private void executeFieldScript(String field) throws IOException { + assert mapperScripts.containsKey(field); + mapperScripts.get(field).script.accept(this); + mapperScripts.get(field).script = c -> {}; + } + + private static class MapperScript { + CheckedConsumer script; + MapperScript(CheckedConsumer script) { + this.script = script; + } + } + + private class LazyDocumentReader extends LeafReader { private final ParseContext.Document document; private final BytesReference sourceBytes; + private final Set calculatedFields; private LeafReaderContext in; - private LazyDocumentReader(ParseContext.Document document, BytesReference sourceBytes) { + private LazyDocumentReader(ParseContext.Document document, BytesReference sourceBytes, Set calculatedFields) { this.document = document; this.sourceBytes = sourceBytes; + this.calculatedFields = calculatedFields; } - private void buildDocValues() { + private void buildDocValues(String field) throws IOException { + if (calculatedFields.contains(field)) { + // this means that a mapper script is referring to another calculated field; + // in which case we need to execute that field first, and then rebuild the + // memory index + executeFieldScript(field); + calculatedFields.remove(field); + this.in = null; + } if (in != null) { return; } @@ -72,37 +127,41 @@ private void buildDocValues() { @Override public NumericDocValues getNumericDocValues(String field) throws IOException { - buildDocValues(); + buildDocValues(field); return in.reader().getNumericDocValues(field); } @Override public BinaryDocValues getBinaryDocValues(String field) throws IOException { - buildDocValues(); + buildDocValues(field); return in.reader().getBinaryDocValues(field); } @Override public SortedDocValues getSortedDocValues(String field) throws IOException { - buildDocValues(); + buildDocValues(field); return in.reader().getSortedDocValues(field); } @Override public SortedNumericDocValues getSortedNumericDocValues(String field) throws IOException { - buildDocValues(); + buildDocValues(field); return in.reader().getSortedNumericDocValues(field); } @Override public SortedSetDocValues getSortedSetDocValues(String field) throws IOException { - buildDocValues(); + buildDocValues(field); return in.reader().getSortedSetDocValues(field); } @Override public FieldInfos getFieldInfos() { - buildDocValues(); + try { + buildDocValues(null); + } catch (IOException e) { + // won't actually happen + } return in.reader().getFieldInfos(); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java b/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java index 409b2655072e2..e8c073562ce1e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java @@ -18,7 +18,6 @@ import org.elasticsearch.index.similarity.SimilarityProvider; import org.elasticsearch.script.ScriptService; -import java.io.IOException; import java.util.Map; import java.util.Objects; import java.util.function.BooleanSupplier; @@ -206,9 +205,4 @@ public final String simpleName() { */ public abstract void validate(MappingLookup mappers); - /** - * Perform mapping actions after a document's source has been parsed - */ - public abstract void postParse(ParseContext context) throws IOException; - } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java index cafcfba49b134..20d6be8bc4967 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -8,10 +8,12 @@ package org.elasticsearch.index.mapper; +import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.analysis.IndexAnalyzers; import org.elasticsearch.index.analysis.NamedAnalyzer; +import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -55,6 +57,7 @@ private CacheKey() {} private final boolean hasNested; private final FieldTypeLookup fieldTypeLookup; private final Map indexAnalyzersMap = new HashMap<>(); + private final Map> postParsePhases = new HashMap<>(); private final DocumentParser documentParser; private final Mapping mapping; private final IndexSettings indexSettings; @@ -137,6 +140,10 @@ public MappingLookup(Mapping mapping, throw new MapperParsingException("Field [" + mapper.name() + "] is defined more than once"); } indexAnalyzersMap.putAll(mapper.indexAnalyzers()); + CheckedConsumer postParsePhase = mapper.getPostParsePhase(); + if (postParsePhase != null) { + postParsePhases.put(mapper.fieldType().name(), postParsePhase); + } } for (FieldAliasMapper aliasMapper : aliasMappers) { @@ -174,6 +181,10 @@ public NamedAnalyzer indexAnalyzer(String field, Function return unmappedFieldAnalyzer.apply(field); } + public Map> getPostParsePhases() { + return postParsePhases; + } + /** * Returns an iterable over all the registered field mappers (including alias mappers) */ diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 3e0f9b82efe6e..52e3dd759be20 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -24,6 +24,7 @@ import org.apache.lucene.search.Query; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; +import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.Numbers; import org.elasticsearch.common.lucene.search.Queries; @@ -78,7 +79,7 @@ public static class Builder extends FieldMapper.Builder { private final Parameter nullValue; - private final Parameter> script; + private final Parameter> script; private final Parameter> meta = Parameter.metaParam(); @@ -150,7 +151,7 @@ public Float parse(Object value, boolean coerce) { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction compiler(String fieldName) { return (s, ss) -> { throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); }; @@ -267,7 +268,7 @@ public Float parse(XContentParser parser, boolean coerce) throws IOException { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction compiler(String fieldName) { return (s, ss) -> { throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); }; @@ -361,7 +362,7 @@ public Double parse(XContentParser parser, boolean coerce) throws IOException { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction compiler(String fieldName) { return (script, service) -> new DoubleMapperScript(fieldName, service.compile(script, DoubleFieldScript.CONTEXT), script.getParams()); } @@ -442,7 +443,7 @@ public Number parsePoint(byte[] value) { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction compiler(String fieldName) { return (s, ss) -> { throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); }; @@ -515,7 +516,7 @@ public Short parse(XContentParser parser, boolean coerce) throws IOException { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction compiler(String fieldName) { return (s, ss) -> { throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); }; @@ -579,7 +580,7 @@ public Integer parse(XContentParser parser, boolean coerce) throws IOException { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction compiler(String fieldName) { return (s, ss) -> { throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); }; @@ -691,7 +692,7 @@ public Long parse(XContentParser parser, boolean coerce) throws IOException { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction compiler(String fieldName) { return (script, service) -> new LongMapperScript(fieldName, service.compile(script, LongFieldScript.CONTEXT), script.getParams()); } @@ -780,7 +781,7 @@ public final NumericType numericType() { public final TypeParser parser() { return parser; } - public abstract BiFunction compiler(String fieldName); + public abstract BiFunction compiler(String fieldName); public abstract Query termQuery(String field, Object value); public abstract Query termsQuery(String field, Collection values); public abstract Query rangeQuery(String field, Object lowerTerm, Object upperTerm, @@ -1060,7 +1061,7 @@ public CollapseType collapseType() { private final Explicit ignoreMalformed; private final Explicit coerce; private final Number nullValue; - private final ScriptParams.CompiledScriptParameter script; + private final ScriptParams.CompiledScriptParameter script; private final boolean ignoreMalformedByDefault; private final boolean coerceByDefault; @@ -1154,11 +1155,11 @@ private void indexValue(ParseContext context, Number numericValue) { } @Override - protected void doPostParse(ParseContext context) { + public CheckedConsumer getPostParsePhase() { if (this.script == null) { - return; + return null; } - this.script.compiledScript.executeAndIndex(context, this::indexValue); + return c -> this.script.compiledScript.executeAndIndex(c, this::indexValue); } @Override @@ -1166,11 +1167,11 @@ public FieldMapper.Builder getMergeBuilder() { return new Builder(simpleName(), type, ignoreMalformedByDefault, coerceByDefault).init(this); } - private interface MapperScript { - void executeAndIndex(ParseContext context, BiConsumer emitter); + private interface NumberMapperScript { + void executeAndIndex(IndexTimeScriptContext context, BiConsumer emitter); } - private static class LongMapperScript implements MapperScript { + private static class LongMapperScript implements NumberMapperScript { final LongFieldScript.Factory scriptFactory; final Map scriptParams; @@ -1183,19 +1184,18 @@ private LongMapperScript(String fieldName, LongFieldScript.Factory scriptFactory } @Override - public void executeAndIndex(ParseContext context, BiConsumer emitter) { - IndexTimeScriptContext scriptContext = context.getScriptContext(); + public void executeAndIndex(IndexTimeScriptContext scriptContext, BiConsumer emitter) { LongFieldScript s = scriptFactory .newFactory(fieldName, scriptParams, scriptContext.searchLookup) .newInstance(scriptContext.leafReaderContext); s.runForDoc(0); for (long v : s.values()) { - emitter.accept(context, v); + emitter.accept(scriptContext.pc, v); } } } - private static class DoubleMapperScript implements MapperScript { + private static class DoubleMapperScript implements NumberMapperScript { final DoubleFieldScript.Factory scriptFactory; final Map scriptParams; @@ -1208,14 +1208,13 @@ private DoubleMapperScript(String fieldName, DoubleFieldScript.Factory scriptFac } @Override - public void executeAndIndex(ParseContext context, BiConsumer emitter) { - IndexTimeScriptContext scriptContext = context.getScriptContext(); + public void executeAndIndex(IndexTimeScriptContext scriptContext, BiConsumer emitter) { DoubleFieldScript s = scriptFactory .newFactory(fieldName, scriptParams, scriptContext.searchLookup) .newInstance(scriptContext.leafReaderContext); s.runForDoc(0); for (double v : s.values()) { - emitter.accept(context, v); + emitter.accept(scriptContext.pc, v); } } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index 844ebfa57646d..3079d9cc82658 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -570,10 +570,4 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep } - @Override - public void postParse(ParseContext context) throws IOException { - for (Mapper mapper : this) { - mapper.postParse(context); - } - } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java b/server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java index 2f8e297264d0d..4ebf0182ef8a0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java @@ -15,9 +15,6 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.analysis.IndexAnalyzers; -import org.elasticsearch.index.fielddata.IndexFieldDataCache; -import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; -import org.elasticsearch.search.lookup.SearchLookup; import java.util.ArrayList; import java.util.Collection; @@ -171,11 +168,6 @@ public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter return in.parserContext(dateFormatter); } - @Override - public IndexTimeScriptContext getScriptContext() { - return in.getScriptContext(); - } - @Override public boolean isWithinCopyTo() { return in.isWithinCopyTo(); @@ -319,7 +311,6 @@ public static class InternalParseContext extends ParseContext { private SeqNoFieldMapper.SequenceIDFields seqID; private long numNestedDocs; private boolean docsReversed = false; - private IndexTimeScriptContext scriptContext = null; public InternalParseContext(MappingLookup mappingLookup, Function parserContextFunction, @@ -341,20 +332,6 @@ public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter return parserContextFunction.apply(dateFormatter); } - @Override - public IndexTimeScriptContext getScriptContext() { - if (scriptContext == null) { - SearchLookup lookup = new SearchLookup( - this.mappingLookup::getFieldType, - (ft, s) -> ft.fielddataBuilder("", s).build( - new IndexFieldDataCache.None(), - new NoneCircuitBreakerService()) - ); - scriptContext = new IndexTimeScriptContext(lookup, document, this.sourceToParse.source()); - } - return scriptContext; - } - @Override public IndexSettings indexSettings() { return this.mappingLookup.getIndexSettings(); @@ -534,8 +511,6 @@ public Collection getIgnoredFields() { public abstract Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter); - public abstract IndexTimeScriptContext getScriptContext(); - /** * Return a new context that will be within a copy-to operation. */ diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index 2e644ad0ea3c5..65c3d6e345faf 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -65,7 +65,7 @@ public void testDoublesAccess() throws IOException { b.startObject("double_field").field("type", "double").endObject(); b.startObject("double_field_plus_two"); b.field("type", "double"); - b.field("script", "plus_two"); + b.field("script", "plus_two_double_field"); b.endObject(); })); @@ -98,10 +98,15 @@ public void testCrossReferences() throws IOException { b.field("type", "long"); b.field("script", "length"); b.endObject(); + b.startObject("message_length_plus_four"); + b.field("type", "double"); + b.field("script", "plus_two_message_length_plus_two"); + b.endObject(); })); ParsedDocument doc = mapper.parse(source(b -> b.field("message", "this is a message"))); assertEquals(doc.rootDoc().getField("message_length_plus_two").numericValue(), 19L); assertEquals(doc.rootDoc().getField("message_length").numericValue(), 17L); + assertEquals(doc.rootDoc().getField("message_length_plus_four").numericValue(), 21d); } public static class TestScriptPlugin extends Plugin implements ScriptPlugin { @@ -152,11 +157,12 @@ public void execute() { } } if (context.factoryClazz == DoubleFieldScript.Factory.class) { - if (code.equals("plus_two")) { + if (code.startsWith("plus_two_")) { + String sourceField = code.substring(9); return (FactoryType) (DoubleFieldScript.Factory) (n, p, l) -> ctx -> new DoubleFieldScript(n, p, l, ctx) { @Override public void execute() { - double input = (double) getDoc().get("double_field").get(0); + double input = ((Number) getDoc().get(sourceField).get(0)).doubleValue(); emit(input + 2); } }; From d81d1a24a695f228007a436265fa76f673703752 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 5 Mar 2021 15:38:10 +0000 Subject: [PATCH 05/39] Add value fetchers; yaml tests --- .../script/ScriptScoreBenchmark.java | 2 +- .../index/mapper/TokenCountFieldMapper.java | 2 +- .../23_long_calculated_at_index.yml | 140 ++++++++++++++ .../33_double_calculated_at_index.yml | 182 ++++++++++++++++++ .../index/mapper/NumberFieldMapper.java | 73 +++++-- .../fielddata/IndexFieldDataServiceTests.java | 4 +- .../index/mapper/NumberFieldTypeTests.java | 2 +- .../aggregations/AggregatorBaseTests.java | 2 +- .../bucket/range/RangeAggregatorTests.java | 3 + .../search/collapse/CollapseBuilderTests.java | 4 +- 10 files changed, 387 insertions(+), 27 deletions(-) create mode 100644 modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml create mode 100644 modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java index b2094084fec30..d0308d2166bfa 100644 --- a/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java @@ -79,7 +79,7 @@ public class ScriptScoreBenchmark { private final ScriptModule scriptModule = new ScriptModule(Settings.EMPTY, pluginsService.filterPlugins(ScriptPlugin.class)); private final Map fieldTypes = Map.ofEntries( - Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of())) + Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null)) ); private final IndexFieldDataCache fieldDataCache = new IndexFieldDataCache.None(); private final CircuitBreakerService breakerService = new NoneCircuitBreakerService(); diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java index 13d36eb2d3217..3d563d97aa18a 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java @@ -77,7 +77,7 @@ static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType { TokenCountFieldType(String name, boolean isSearchable, boolean isStored, boolean hasDocValues, Number nullValue, Map meta) { - super(name, NumberFieldMapper.NumberType.INTEGER, isSearchable, isStored, hasDocValues, false, nullValue, meta); + super(name, NumberFieldMapper.NumberType.INTEGER, isSearchable, isStored, hasDocValues, false, nullValue, meta, null); } @Override diff --git a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml new file mode 100644 index 0000000000000..b7f4b85814488 --- /dev/null +++ b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml @@ -0,0 +1,140 @@ +--- +setup: + - do: + indices.create: + index: sensor + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + mappings: + properties: + timestamp: + type: date + temperature: + type: long + voltage: + type: double + node: + type: keyword + voltage_times_ten: + type: long + script: + source: | + for (double v : doc['voltage']) { + emit((long)(v * params.multiplier)); + } + params: + multiplier: 10 + # Test fetching many values + temperature_digits: + type: long + script: + source: | + for (long temperature : doc['temperature']) { + long t = temperature; + while (t != 0) { + emit(t % 10); + t /= 10; + } + } + + - do: + bulk: + index: sensor + refresh: true + body: | + {"index":{}} + {"timestamp": 1516729294000, "temperature": 200, "voltage": 5.2, "node": "a"} + {"index":{}} + {"timestamp": 1516642894000, "temperature": 201, "voltage": 5.8, "node": "b"} + {"index":{}} + {"timestamp": 1516556494000, "temperature": 202, "voltage": 5.1, "node": "a"} + {"index":{}} + {"timestamp": 1516470094000, "temperature": 198, "voltage": 5.6, "node": "b"} + {"index":{}} + {"timestamp": 1516383694000, "temperature": 200, "voltage": 4.2, "node": "c"} + {"index":{}} + {"timestamp": 1516297294000, "temperature": 202, "voltage": 4.0, "node": "c"} + +--- +"get mapping": + - do: + indices.get_mapping: + index: sensor + - match: {sensor.mappings.properties.voltage_times_ten.type: long } + - match: + sensor.mappings.properties.voltage_times_ten.script.source: | + for (double v : doc['voltage']) { + emit((long)(v * params.multiplier)); + } + - match: {sensor.mappings.properties.voltage_times_ten.script.params: {multiplier: 10} } + - match: {sensor.mappings.properties.voltage_times_ten.script.lang: painless } + +--- +"fetch fields": + - do: + search: + index: sensor + body: + sort: timestamp + fields: + - voltage_times_ten + - temperature_digits + - match: {hits.total.value: 6} + - match: {hits.hits.0.fields.voltage_times_ten: [40] } + - match: {hits.hits.0.fields.temperature_digits: [0, 2, 2] } + - match: {hits.hits.0.fields.voltage_times_ten: [40] } + - match: {hits.hits.1.fields.voltage_times_ten: [42] } + - match: {hits.hits.2.fields.voltage_times_ten: [56] } + - match: {hits.hits.3.fields.voltage_times_ten: [51] } + - match: {hits.hits.4.fields.voltage_times_ten: [58] } + - match: {hits.hits.5.fields.voltage_times_ten: [52] } + +--- +"docvalue_fields": + - do: + search: + index: sensor + body: + sort: timestamp + docvalue_fields: + - voltage_times_ten + - temperature_digits + - match: {hits.total.value: 6} + - match: {hits.hits.0.fields.voltage_times_ten: [40] } + - match: {hits.hits.0.fields.temperature_digits: [0, 2, 2] } + - match: {hits.hits.0.fields.voltage_times_ten: [40] } + - match: {hits.hits.1.fields.voltage_times_ten: [42] } + - match: {hits.hits.2.fields.voltage_times_ten: [56] } + - match: {hits.hits.3.fields.voltage_times_ten: [51] } + - match: {hits.hits.4.fields.voltage_times_ten: [58] } + - match: {hits.hits.5.fields.voltage_times_ten: [52] } + +--- +"terms agg": + - do: + search: + index: sensor + body: + aggs: + v10: + terms: + field: voltage_times_ten + - match: {hits.total.value: 6} + - match: {aggregations.v10.buckets.0.key: 40.0} + - match: {aggregations.v10.buckets.0.doc_count: 1} + - match: {aggregations.v10.buckets.1.key: 42.0} + - match: {aggregations.v10.buckets.1.doc_count: 1} + +--- +"term query": + - do: + search: + index: sensor + body: + query: + term: + voltage_times_ten: 58 + - match: {hits.total.value: 1} + - match: {hits.hits.0._source.voltage: 5.8} diff --git a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml new file mode 100644 index 0000000000000..be9f3bfeac016 --- /dev/null +++ b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml @@ -0,0 +1,182 @@ +--- +setup: + - do: + indices.create: + index: sensor + body: + settings: + number_of_shards: 1 + number_of_replicas: 0 + mappings: + properties: + timestamp: + type: date + temperature: + type: long + voltage: + type: double + node: + type: keyword + voltage_percent: + type: double + script: + source: | + for (double v : doc['voltage']) { + emit(v / params.max); + } + params: + max: 5.8 + # Test fetching many values + voltage_sqrts: + type: double + script: + source: | + for (double voltage : doc['voltage']) { + double v = voltage; + while (v > 1.2) { + emit(v); + v = Math.sqrt(v); + } + } + + - do: + bulk: + index: sensor + refresh: true + body: | + {"index":{}} + {"timestamp": 1516729294000, "temperature": 200, "voltage": 5.2, "node": "a"} + {"index":{}} + {"timestamp": 1516642894000, "temperature": 201, "voltage": 5.8, "node": "b"} + {"index":{}} + {"timestamp": 1516556494000, "temperature": 202, "voltage": 5.1, "node": "a"} + {"index":{}} + {"timestamp": 1516470094000, "temperature": 198, "voltage": 5.6, "node": "b"} + {"index":{}} + {"timestamp": 1516383694000, "temperature": 200, "voltage": 4.2, "node": "c"} + {"index":{}} + {"timestamp": 1516297294000, "temperature": 202, "voltage": 4.0, "node": "c"} + +--- +"get mapping": + - do: + indices.get_mapping: + index: sensor + - match: {sensor.mappings.properties.voltage_percent.type: double } + - match: + sensor.mappings.properties.voltage_percent.script.source: | + for (double v : doc['voltage']) { + emit(v / params.max); + } + - match: {sensor.mappings.properties.voltage_percent.script.params: {max: 5.8} } + - match: {sensor.mappings.properties.voltage_percent.script.lang: painless } + +--- +"fetch fields": + - do: + search: + index: sensor + body: + sort: timestamp + fields: [voltage_percent, voltage_sqrts] + - match: {hits.total.value: 6} + - match: {hits.hits.0.fields.voltage_percent: [0.6896551724137931] } + # 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] } + - match: {hits.hits.2.fields.voltage_percent: [0.9655172413793103] } + - match: {hits.hits.3.fields.voltage_percent: [0.8793103448275862] } + - match: {hits.hits.4.fields.voltage_percent: [1.0] } + - match: {hits.hits.5.fields.voltage_percent: [0.896551724137931] } + +--- +"docvalue_fields": + - do: + search: + index: sensor + body: + sort: timestamp + docvalue_fields: [voltage_percent, voltage_sqrts] + - match: {hits.total.value: 6} + - match: {hits.hits.0.fields.voltage_percent: [0.6896551724137931] } + # 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] } + - match: {hits.hits.2.fields.voltage_percent: [0.9655172413793103] } + - match: {hits.hits.3.fields.voltage_percent: [0.8793103448275862] } + - match: {hits.hits.4.fields.voltage_percent: [1.0] } + - match: {hits.hits.5.fields.voltage_percent: [0.896551724137931] } + +--- +"terms agg": + - do: + search: + index: sensor + body: + aggs: + v10: + terms: + field: voltage_percent + - match: {hits.total.value: 6} + - match: {aggregations.v10.buckets.0.key: 0.6896551724137931} + - match: {aggregations.v10.buckets.0.doc_count: 1} + - match: {aggregations.v10.buckets.1.key: 0.7241379310344828} + - match: {aggregations.v10.buckets.1.doc_count: 1} + +--- +"range query": + - do: + search: + index: sensor + body: + query: + range: + voltage_percent: + lt: .7 + - match: {hits.total.value: 1} + - match: {hits.hits.0._source.voltage: 4.0} + + - do: + search: + index: sensor + body: + query: + range: + voltage_percent: + gt: 1 + - match: {hits.total.value: 0} + + - do: + search: + index: sensor + body: + query: + range: + voltage_percent: + gte: 1 + - match: {hits.total.value: 1} + - match: {hits.hits.0._source.voltage: 5.8} + + - do: + search: + index: sensor + body: + query: + range: + voltage_percent: + gte: .7 + lte: .8 + - match: {hits.total.value: 1} + - match: {hits.hits.0._source.voltage: 4.2} + +--- +"term query": + - do: + search: + index: sensor + body: + query: + term: + voltage_percent: 1.0 + - match: {hits.total.value: 1} + - match: {hits.hits.0._source.voltage: 5.8} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 52e3dd759be20..d793cdd73c8ad 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -18,6 +18,7 @@ import org.apache.lucene.document.LongPoint; import org.apache.lucene.document.SortedNumericDocValuesField; import org.apache.lucene.document.StoredField; +import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.IndexSortSortedNumericDocValuesRangeQuery; import org.apache.lucene.search.MatchNoDocsQuery; @@ -43,18 +44,19 @@ import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.lookup.SearchLookup; +import org.elasticsearch.search.lookup.SourceLookup; import java.io.IOException; import java.time.ZoneId; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.function.BiConsumer; import java.util.function.BiFunction; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; @@ -945,22 +947,26 @@ public static class NumberFieldType extends SimpleMappedFieldType { private final NumberType type; private final boolean coerce; private final Number nullValue; + private final NumberMapperScript script; public NumberFieldType(String name, NumberType type, boolean isSearchable, boolean isStored, - boolean hasDocValues, boolean coerce, Number nullValue, Map meta) { + boolean hasDocValues, boolean coerce, Number nullValue, Map meta, + ScriptParams.CompiledScriptParameter script) { super(name, isSearchable, isStored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta); this.type = Objects.requireNonNull(type); this.coerce = coerce; this.nullValue = nullValue; + this.script = script == null ? null : script.compiledScript; } NumberFieldType(String name, Builder builder) { this(name, builder.type, builder.indexed.getValue(), builder.stored.getValue(), builder.hasDocValues.getValue(), - builder.coerce.getValue().value(), builder.nullValue.getValue(), builder.meta.getValue()); + builder.coerce.getValue().value(), builder.nullValue.getValue(), builder.meta.getValue(), + builder.script.get()); } public NumberFieldType(String name, NumberType type) { - this(name, type, true, false, true, true, null, Collections.emptyMap()); + this(name, type, true, false, true, true, null, null, null); } @Override @@ -1018,6 +1024,28 @@ public ValueFetcher valueFetcher(SearchExecutionContext context, String format) if (format != null) { throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats."); } + if (this.script != null) { + // values don't live in source - either pull from dv, or re-calculate + if (hasDocValues()) { + return new DocValueFetcher(DocValueFormat.RAW, context.getForField(this)); + } + return new ValueFetcher() { + LeafReaderContext ctx; + + @Override + public void setNextReader(LeafReaderContext context) { + this.ctx = context; + } + + @Override + public List fetchValues(SourceLookup lookup) { + List values = new ArrayList<>(); + script.executeAndIndex(context.lookup(), ctx, lookup.docId(), values::add); + values.sort(Comparator.comparingLong(a -> (Long) a)); + return values; + } + }; + } return new SourceValueFetcher(name(), context, nullValue) { @Override @@ -1159,7 +1187,12 @@ public CheckedConsumer getPostParsePhase() if (this.script == null) { return null; } - return c -> this.script.compiledScript.executeAndIndex(c, this::indexValue); + return c -> this.script.compiledScript.executeAndIndex( + c.searchLookup, + c.leafReaderContext, + 0, + v -> this.indexValue(c.pc, v) + ); } @Override @@ -1168,7 +1201,7 @@ public FieldMapper.Builder getMergeBuilder() { } private interface NumberMapperScript { - void executeAndIndex(IndexTimeScriptContext context, BiConsumer emitter); + void executeAndIndex(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter); } private static class LongMapperScript implements NumberMapperScript { @@ -1184,13 +1217,14 @@ private LongMapperScript(String fieldName, LongFieldScript.Factory scriptFactory } @Override - public void executeAndIndex(IndexTimeScriptContext scriptContext, BiConsumer emitter) { + public void executeAndIndex(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { LongFieldScript s = scriptFactory - .newFactory(fieldName, scriptParams, scriptContext.searchLookup) - .newInstance(scriptContext.leafReaderContext); - s.runForDoc(0); - for (long v : s.values()) { - emitter.accept(scriptContext.pc, v); + .newFactory(fieldName, scriptParams, lookup) + .newInstance(ctx); + s.runForDoc(doc); + long[] vs = s.values(); + for (int i = 0; i < s.count(); i++) { + emitter.accept(vs[i]); } } } @@ -1208,13 +1242,14 @@ private DoubleMapperScript(String fieldName, DoubleFieldScript.Factory scriptFac } @Override - public void executeAndIndex(IndexTimeScriptContext scriptContext, BiConsumer emitter) { + public void executeAndIndex(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { DoubleFieldScript s = scriptFactory - .newFactory(fieldName, scriptParams, scriptContext.searchLookup) - .newInstance(scriptContext.leafReaderContext); - s.runForDoc(0); - for (double v : s.values()) { - emitter.accept(scriptContext.pc, v); + .newFactory(fieldName, scriptParams, lookup) + .newInstance(ctx); + s.runForDoc(doc); + double[] vs = s.values(); + for (int i = 0; i < s.count(); i++) { + emitter.accept(vs[i]); } } } diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java index 0f47520cca0c8..8d547b66f1bf2 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java @@ -303,13 +303,13 @@ private void doTestRequireDocValues(MappedFieldType ft) { public void testRequireDocValuesOnLongs() { doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.LONG)); doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.LONG, - true, false, false, false, null, Collections.emptyMap())); + true, false, false, false, null, Collections.emptyMap(), null)); } public void testRequireDocValuesOnDoubles() { doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.DOUBLE)); doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.DOUBLE, - true, false, false, false, null, Collections.emptyMap())); + true, false, false, false, null, Collections.emptyMap(), null)); } public void testRequireDocValuesOnBools() { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java index 27b37a0e8bf7c..c2037591fc268 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java @@ -122,7 +122,7 @@ public void testLongTermQueryWithDecimalPart() { } private static MappedFieldType unsearchable() { - return new NumberFieldType("field", NumberType.LONG, false, false, true, true, null, Collections.emptyMap()); + return new NumberFieldType("field", NumberType.LONG, false, false, true, true, null, Collections.emptyMap(), null); } public void testTermQuery() { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorBaseTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorBaseTests.java index 8341b66b2f2fe..52ce6dd0034b9 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorBaseTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorBaseTests.java @@ -80,7 +80,7 @@ private ValuesSourceConfig getVSConfig( AggregationContext context ) { MappedFieldType ft - = new NumberFieldMapper.NumberFieldType(fieldName, numType, indexed, false, true, false, null, Collections.emptyMap()); + = new NumberFieldMapper.NumberFieldType(fieldName, numType, indexed, false, true, false, null, Collections.emptyMap(), null); return ValuesSourceConfig.resolveFieldOnly(ft, context); } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorTests.java index b0eed25079e4a..e1670c8013f84 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorTests.java @@ -132,6 +132,7 @@ public void testUnboundedRanges() throws IOException { true, false, null, + null, null ) ); @@ -225,6 +226,7 @@ public void testNotFitIntoDouble() throws IOException { true, false, null, + null, null ); @@ -416,6 +418,7 @@ private void testCase(Query query, true, false, null, + null, null ); RangeAggregationBuilder aggregationBuilder = new RangeAggregationBuilder("test_range_agg"); diff --git a/server/src/test/java/org/elasticsearch/search/collapse/CollapseBuilderTests.java b/server/src/test/java/org/elasticsearch/search/collapse/CollapseBuilderTests.java index 16aef6da8f34b..aba60f06d7cb1 100644 --- a/server/src/test/java/org/elasticsearch/search/collapse/CollapseBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/collapse/CollapseBuilderTests.java @@ -145,14 +145,14 @@ public void testBuild() throws IOException { numberFieldType = new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.LONG, true, false, - false, false, null, Collections.emptyMap()); + false, false, null, Collections.emptyMap(), null); when(searchExecutionContext.getFieldType("field")).thenReturn(numberFieldType); IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> builder.build(searchExecutionContext)); assertEquals(exc.getMessage(), "cannot collapse on field `field` without `doc_values`"); numberFieldType = new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.LONG, false, false, - true, false, null, Collections.emptyMap()); + true, false, null, Collections.emptyMap(), null); when(searchExecutionContext.getFieldType("field")).thenReturn(numberFieldType); builder.setInnerHits(new InnerHitBuilder()); exc = expectThrows(IllegalArgumentException.class, () -> builder.build(searchExecutionContext)); From 51946a88ca46aa28b342324f10298fbfa2c7980c Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 9 Mar 2021 13:48:48 +0000 Subject: [PATCH 06/39] Avoid null metadata --- .../java/org/elasticsearch/index/mapper/NumberFieldMapper.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index d793cdd73c8ad..aa36cfdfed458 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -51,6 +51,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Map; @@ -966,7 +967,7 @@ public NumberFieldType(String name, NumberType type, boolean isSearchable, boole } public NumberFieldType(String name, NumberType type) { - this(name, type, true, false, true, true, null, null, null); + this(name, type, true, false, true, true, null, Collections.emptyMap(), null); } @Override From d4913be0fff93b3ffafd34350529140f998c774e Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 10 Mar 2021 15:32:34 +0000 Subject: [PATCH 07/39] Pass index name; throw error if you try and index into a script field --- .../index/mapper/IndexTimeScriptContext.java | 2 +- .../index/mapper/NumberFieldMapper.java | 5 +++ .../index/mapper/CalculatedFieldTests.java | 42 +++++++++++++++++++ .../index/mapper/TestRuntimeField.java | 2 +- 4 files changed, 49 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java b/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java index 07f56501d5e74..cb7b5ea60b90f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java @@ -58,7 +58,7 @@ public static void executePostParsePhases(MappingLookup lookup, ParseContext par private IndexTimeScriptContext(MappingLookup mappingLookup, ParseContext pc) { this.searchLookup = new SearchLookup( mappingLookup::getFieldType, - (ft, s) -> ft.fielddataBuilder("", s).build( + (ft, s) -> ft.fielddataBuilder(pc.indexSettings().getIndex().getName(), s).build( new IndexFieldDataCache.None(), new NoneCircuitBreakerService()) ); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index aa36cfdfed458..3b5f98c540f80 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -1134,6 +1134,11 @@ protected String contentType() { @Override protected void parseCreateField(ParseContext context) throws IOException { + + if (this.script != null) { + throw new IllegalArgumentException("Cannot index data directly into scripted field"); + } + XContentParser parser = context.parser(); Object value; Number numericValue = null; diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index 65c3d6e345faf..ee3739bdbfa7c 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -17,6 +17,7 @@ import org.elasticsearch.runtimefields.mapper.LongFieldScript; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptEngine; +import org.junit.Ignore; import java.io.IOException; import java.util.Collection; @@ -109,6 +110,40 @@ public void testCrossReferences() throws IOException { assertEquals(doc.rootDoc().getField("message_length_plus_four").numericValue(), 21d); } + public void testCannotIndexDirectlyIntoScriptMapper() throws IOException { + DocumentMapper mapper = createDocumentMapper(mapping(b -> { + b.startObject("message").field("type", "text").endObject(); + b.startObject("message_length"); + b.field("type", "long"); + b.field("script", "length"); + b.endObject(); + })); + + Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> { + b.field("message", "foo"); + b.field("message_length", 3); + }))); + assertEquals("failed to parse field [message_length] of type [long] in document with id '1'. Preview of field's value: '3'", + e.getMessage()); + Throwable original = e.getCause(); + assertEquals("Cannot index data directly into scripted field", original.getMessage()); + } + + @Ignore("This currently doesn't fail properly - should it?") + public void testCannotReferToRuntimeFields() throws IOException { + DocumentMapper mapper = createDocumentMapper(topMapping(b -> { + b.startObject("runtime"); + b.startObject("runtime-field").field("type", "long").endObject(); + b.endObject(); + b.startObject("properties"); + b.startObject("index-field").field("type", "long").field("script", "refer_to_runtime").endObject(); + b.endObject(); + })); + + Exception e = expectThrows(IllegalArgumentException.class, () -> mapper.parse(source(b -> {}))); + assertEquals("Cannot reference runtime field [runtime-field] in an index-time script", e.getMessage()); + } + public static class TestScriptPlugin extends Plugin implements ScriptPlugin { @Override @@ -154,6 +189,13 @@ public void execute() { emit(input + 2); } }; + case "refer_to_runtime": + return (FactoryType) (LongFieldScript.Factory) (n, p, l) -> ctx -> new LongFieldScript(n, p, l, ctx) { + @Override + public void execute() { + emit((long) getDoc().get("runtime-field").get(0)); + } + }; } } if (context.factoryClazz == DoubleFieldScript.Factory.class) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/TestRuntimeField.java b/server/src/test/java/org/elasticsearch/index/mapper/TestRuntimeField.java index fd359fe1bde14..af49d76d125dd 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/TestRuntimeField.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/TestRuntimeField.java @@ -20,7 +20,7 @@ public class TestRuntimeField extends RuntimeFieldType { private final String type; public TestRuntimeField(String name, String type) { - super(name, Collections.emptyMap(), null); + super(name, Collections.emptyMap(), (b, p) -> {}); this.type = type; } From 5331ae50cfc1e3ff91cad9bb0483b1b59ee43838 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 15 Mar 2021 12:15:18 +0000 Subject: [PATCH 08/39] Rename, consolidate, add javadocs --- .../index/mapper/DocumentParser.java | 2 +- .../index/mapper/FieldMapper.java | 2 +- .../index/mapper/MapperScript.java | 64 ++++++++++++++++++ .../index/mapper/MappingLookup.java | 8 +-- .../index/mapper/NumberFieldMapper.java | 65 ++++++++++--------- ...riptContext.java => PostParseContext.java} | 40 +++++++----- .../index/mapper/PostParseExecutor.java | 17 +++++ .../index/mapper/ScriptParams.java | 55 ++++------------ .../index/mapper/CalculatedFieldTests.java | 3 +- 9 files changed, 160 insertions(+), 96 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/MapperScript.java rename server/src/main/java/org/elasticsearch/index/mapper/{IndexTimeScriptContext.java => PostParseContext.java} (88%) create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 6832370564d88..d787133018900 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -108,7 +108,7 @@ private static void internalParseDocument(MappingLookup lookup, MetadataFieldMap parseObjectOrNested(context, root); } - IndexTimeScriptContext.executePostParsePhases(lookup, context); + PostParseContext.executePostParsePhases(lookup, context); for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) { metadataMapper.postParse(context); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index 1e9506c69b5e1..22cf2ab81a25e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -171,7 +171,7 @@ public void parse(ParseContext context) throws IOException { /** * Returns a post-parse executor for this field mapper, or {@code null} if none is defined */ - public CheckedConsumer getPostParsePhase() { + public PostParseExecutor getPostParseExecutor() { return null; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperScript.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperScript.java new file mode 100644 index 0000000000000..0f98734f682ab --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperScript.java @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.index.mapper; + +import org.apache.lucene.index.LeafReaderContext; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.script.Script; +import org.elasticsearch.search.lookup.SearchLookup; + +import java.io.IOException; +import java.util.Objects; +import java.util.function.Consumer; + +/** + * A script executed to populate values on a FieldMapper + * @param the type of object indexed by the mapper + */ +public abstract class MapperScript implements ToXContentObject { + + private final Script script; + + protected MapperScript(Script script) { + this.script = script; + } + + /** + * Execute the mapper script and emit its results + * @param lookup a SearchLookup for use by the script + * @param ctx a LeafReaderContext for use by the script + * @param doc the document to retrieve values for + * @param emitter a consumer to receive the retrieved values + */ + public abstract void executeAndEmit(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter); + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + MapperScript that = (MapperScript) o; + return Objects.equals(script, that.script); + } + + @Override + public int hashCode() { + return Objects.hash(script); + } + + @Override + public String toString() { + return script.toString(); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return script.toXContent(builder, params); + } +} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java index ed636863d076f..88588280bc6f0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -8,12 +8,10 @@ package org.elasticsearch.index.mapper; -import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.analysis.IndexAnalyzers; import org.elasticsearch.index.analysis.NamedAnalyzer; -import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -57,7 +55,7 @@ private CacheKey() {} private final boolean hasNested; private final FieldTypeLookup fieldTypeLookup; private final Map indexAnalyzersMap = new HashMap<>(); - private final Map> postParsePhases = new HashMap<>(); + private final Map postParsePhases = new HashMap<>(); private final DocumentParser documentParser; private final Mapping mapping; private final IndexSettings indexSettings; @@ -140,7 +138,7 @@ public MappingLookup(Mapping mapping, throw new MapperParsingException("Field [" + mapper.name() + "] is defined more than once"); } indexAnalyzersMap.putAll(mapper.indexAnalyzers()); - CheckedConsumer postParsePhase = mapper.getPostParsePhase(); + PostParseExecutor postParsePhase = mapper.getPostParseExecutor(); if (postParsePhase != null) { postParsePhases.put(mapper.fieldType().name(), postParsePhase); } @@ -181,7 +179,7 @@ public NamedAnalyzer indexAnalyzer(String field, Function return unmappedFieldAnalyzer.apply(field); } - public Map> getPostParsePhases() { + public Map getPostParsePhases() { return postParsePhases; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 3b5f98c540f80..7ce527dffa856 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -25,7 +25,6 @@ import org.apache.lucene.search.Query; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; -import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.Explicit; import org.elasticsearch.common.Numbers; import org.elasticsearch.common.lucene.search.Queries; @@ -82,7 +81,7 @@ public static class Builder extends FieldMapper.Builder { private final Parameter nullValue; - private final Parameter> script; + private final Parameter> script; private final Parameter> meta = Parameter.metaParam(); @@ -154,7 +153,7 @@ public Float parse(Object value, boolean coerce) { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction> compiler(String fieldName) { return (s, ss) -> { throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); }; @@ -271,7 +270,7 @@ public Float parse(XContentParser parser, boolean coerce) throws IOException { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction> compiler(String fieldName) { return (s, ss) -> { throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); }; @@ -365,9 +364,9 @@ public Double parse(XContentParser parser, boolean coerce) throws IOException { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction> compiler(String fieldName) { return (script, service) - -> new DoubleMapperScript(fieldName, service.compile(script, DoubleFieldScript.CONTEXT), script.getParams()); + -> new DoubleMapperScript(fieldName, script, service.compile(script, DoubleFieldScript.CONTEXT), script.getParams()); } @Override @@ -446,7 +445,7 @@ public Number parsePoint(byte[] value) { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction> compiler(String fieldName) { return (s, ss) -> { throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); }; @@ -519,7 +518,7 @@ public Short parse(XContentParser parser, boolean coerce) throws IOException { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction> compiler(String fieldName) { return (s, ss) -> { throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); }; @@ -583,7 +582,7 @@ public Integer parse(XContentParser parser, boolean coerce) throws IOException { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction> compiler(String fieldName) { return (s, ss) -> { throw new IllegalArgumentException("Unknown parameter [script] for mapper [" + fieldName + "]"); }; @@ -695,9 +694,9 @@ public Long parse(XContentParser parser, boolean coerce) throws IOException { } @Override - public BiFunction compiler(String fieldName) { + public BiFunction> compiler(String fieldName) { return (script, service) - -> new LongMapperScript(fieldName, service.compile(script, LongFieldScript.CONTEXT), script.getParams()); + -> new LongMapperScript(fieldName, script, service.compile(script, LongFieldScript.CONTEXT), script.getParams()); } @Override @@ -784,7 +783,7 @@ public final NumericType numericType() { public final TypeParser parser() { return parser; } - public abstract BiFunction compiler(String fieldName); + public abstract BiFunction> compiler(String fieldName); public abstract Query termQuery(String field, Object value); public abstract Query termsQuery(String field, Collection values); public abstract Query rangeQuery(String field, Object lowerTerm, Object upperTerm, @@ -948,16 +947,16 @@ public static class NumberFieldType extends SimpleMappedFieldType { private final NumberType type; private final boolean coerce; private final Number nullValue; - private final NumberMapperScript script; + private final MapperScript script; public NumberFieldType(String name, NumberType type, boolean isSearchable, boolean isStored, boolean hasDocValues, boolean coerce, Number nullValue, Map meta, - ScriptParams.CompiledScriptParameter script) { + MapperScript script) { super(name, isSearchable, isStored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta); this.type = Objects.requireNonNull(type); this.coerce = coerce; this.nullValue = nullValue; - this.script = script == null ? null : script.compiledScript; + this.script = script == null ? null : script; } NumberFieldType(String name, Builder builder) { @@ -1041,7 +1040,7 @@ public void setNextReader(LeafReaderContext context) { @Override public List fetchValues(SourceLookup lookup) { List values = new ArrayList<>(); - script.executeAndIndex(context.lookup(), ctx, lookup.docId(), values::add); + script.executeAndEmit(context.lookup(), ctx, lookup.docId(), values::add); values.sort(Comparator.comparingLong(a -> (Long) a)); return values; } @@ -1090,7 +1089,7 @@ public CollapseType collapseType() { private final Explicit ignoreMalformed; private final Explicit coerce; private final Number nullValue; - private final ScriptParams.CompiledScriptParameter script; + private final MapperScript script; private final boolean ignoreMalformedByDefault; private final boolean coerceByDefault; @@ -1189,11 +1188,11 @@ private void indexValue(ParseContext context, Number numericValue) { } @Override - public CheckedConsumer getPostParsePhase() { + public PostParseExecutor getPostParseExecutor() { if (this.script == null) { return null; } - return c -> this.script.compiledScript.executeAndIndex( + return c -> this.script.executeAndEmit( c.searchLookup, c.leafReaderContext, 0, @@ -1206,24 +1205,26 @@ public FieldMapper.Builder getMergeBuilder() { return new Builder(simpleName(), type, ignoreMalformedByDefault, coerceByDefault).init(this); } - private interface NumberMapperScript { - void executeAndIndex(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter); - } - - private static class LongMapperScript implements NumberMapperScript { + private static class LongMapperScript extends MapperScript { final LongFieldScript.Factory scriptFactory; final Map scriptParams; final String fieldName; - private LongMapperScript(String fieldName, LongFieldScript.Factory scriptFactory, Map scriptParams) { + private LongMapperScript( + String fieldName, + Script script, + LongFieldScript.Factory scriptFactory, + Map scriptParams + ) { + super(script); this.scriptFactory = scriptFactory; this.scriptParams = scriptParams; this.fieldName = fieldName; } @Override - public void executeAndIndex(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { + public void executeAndEmit(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { LongFieldScript s = scriptFactory .newFactory(fieldName, scriptParams, lookup) .newInstance(ctx); @@ -1235,20 +1236,26 @@ public void executeAndIndex(SearchLookup lookup, LeafReaderContext ctx, int doc, } } - private static class DoubleMapperScript implements NumberMapperScript { + private static class DoubleMapperScript extends MapperScript { final DoubleFieldScript.Factory scriptFactory; final Map scriptParams; final String fieldName; - private DoubleMapperScript(String fieldName, DoubleFieldScript.Factory scriptFactory, Map scriptParams) { + private DoubleMapperScript( + String fieldName, + Script script, + DoubleFieldScript.Factory scriptFactory, + Map scriptParams + ) { + super(script); this.scriptFactory = scriptFactory; this.scriptParams = scriptParams; this.fieldName = fieldName; } @Override - public void executeAndIndex(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { + public void executeAndEmit(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { DoubleFieldScript s = scriptFactory .newFactory(fieldName, scriptParams, lookup) .newInstance(ctx); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java similarity index 88% rename from server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java rename to server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java index cb7b5ea60b90f..f259efb75447b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScriptContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java @@ -28,7 +28,6 @@ import org.apache.lucene.index.Terms; import org.apache.lucene.index.memory.MemoryIndex; import org.apache.lucene.util.Bits; -import org.elasticsearch.common.CheckedConsumer; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.index.fielddata.IndexFieldDataCache; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; @@ -40,22 +39,22 @@ import java.util.Map; import java.util.Set; -public class IndexTimeScriptContext { +public class PostParseContext { public final SearchLookup searchLookup; public final LeafReaderContext leafReaderContext; public final ParseContext pc; - private final Map mapperScripts = new HashMap<>(); + private final Map fieldExecutors = new HashMap<>(); public static void executePostParsePhases(MappingLookup lookup, ParseContext parseContext) throws IOException { if (lookup.getPostParsePhases().isEmpty()) { return; } - IndexTimeScriptContext context = new IndexTimeScriptContext(lookup, parseContext); + PostParseContext context = new PostParseContext(lookup, parseContext); context.executePostParse(); } - private IndexTimeScriptContext(MappingLookup mappingLookup, ParseContext pc) { + private PostParseContext(MappingLookup mappingLookup, ParseContext pc) { this.searchLookup = new SearchLookup( mappingLookup::getFieldType, (ft, s) -> ft.fielddataBuilder(pc.indexSettings().getIndex().getName(), s).build( @@ -68,25 +67,32 @@ private IndexTimeScriptContext(MappingLookup mappingLookup, ParseContext pc) { pc.sourceToParse().source(), mappingLookup.getPostParsePhases().keySet()); this.leafReaderContext = reader.getContext(); - mappingLookup.getPostParsePhases().forEach((k, c) -> mapperScripts.put(k, new MapperScript(c))); + mappingLookup.getPostParsePhases().forEach((k, c) -> fieldExecutors.put(k, new FieldExecutor(c))); } private void executePostParse() throws IOException { - for (MapperScript ms : mapperScripts.values()) { - ms.script.accept(this); + for (FieldExecutor ms : fieldExecutors.values()) { + ms.execute(); } } - private void executeFieldScript(String field) throws IOException { - assert mapperScripts.containsKey(field); - mapperScripts.get(field).script.accept(this); - mapperScripts.get(field).script = c -> {}; + private void executeField(String field) throws IOException { + assert fieldExecutors.containsKey(field); + fieldExecutors.get(field).execute(); } - private static class MapperScript { - CheckedConsumer script; - MapperScript(CheckedConsumer script) { - this.script = script; + private class FieldExecutor { + PostParseExecutor executor; + boolean executed = false; + FieldExecutor(PostParseExecutor executor) { + this.executor = executor; + } + + void execute() throws IOException { + if (executed == false) { + executor.execute(PostParseContext.this); + executed = true; + } } } @@ -108,7 +114,7 @@ private void buildDocValues(String field) throws IOException { // this means that a mapper script is referring to another calculated field; // in which case we need to execute that field first, and then rebuild the // memory index - executeFieldScript(field); + executeField(field); calculatedFields.remove(field); this.in = null; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java new file mode 100644 index 0000000000000..a9e40aa4dc51c --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java @@ -0,0 +1,17 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.index.mapper; + +import java.io.IOException; + +public interface PostParseExecutor { + + void execute(PostParseContext context) throws IOException; + +} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java b/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java index 76d49115e2c04..efeda8d9ec8f9 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java @@ -8,56 +8,29 @@ package org.elasticsearch.index.mapper; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService; -import java.io.IOException; -import java.util.Objects; import java.util.function.BiFunction; import java.util.function.Function; +/** + * Static methods for mapping parameters relating to mapper scripts + */ public final class ScriptParams { private ScriptParams() { } - public static class CompiledScriptParameter implements ToXContent { - final Script script; - final T compiledScript; - - public CompiledScriptParameter(Script script, T compiledScript) { - this.script = script; - this.compiledScript = compiledScript; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - CompiledScriptParameter that = (CompiledScriptParameter) o; - return Objects.equals(script, that.script); - } - - @Override - public int hashCode() { - return Objects.hash(script); - } - - @Override - public String toString() { - return script.toString(); - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return script.toXContent(builder, params); - } - } - - public static FieldMapper.Parameter> script( - BiFunction compiler, - Function>initializer + /** + * Defines a script parameter + * @param compiler a function that produces a compiled script, given a {@link Script} and {@link ScriptService} + * @param initializer retrieves the equivalent parameter from an existing FieldMapper for use in merges + * @param the type of the compiled script + * @return a script parameter + */ + public static FieldMapper.Parameter> script( + BiFunction> compiler, + Function>initializer ) { return new FieldMapper.Parameter<>( "script", @@ -68,7 +41,7 @@ public static FieldMapper.Parameter> script( return null; } Script script = Script.parse(o); - return new CompiledScriptParameter<>(script, compiler.apply(script, c.scriptService())); + return compiler.apply(script, c.scriptService()); }, initializer ).acceptsNull(); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index ee3739bdbfa7c..29e6da6bb172f 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -17,7 +17,6 @@ import org.elasticsearch.runtimefields.mapper.LongFieldScript; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptEngine; -import org.junit.Ignore; import java.io.IOException; import java.util.Collection; @@ -129,7 +128,7 @@ public void testCannotIndexDirectlyIntoScriptMapper() throws IOException { assertEquals("Cannot index data directly into scripted field", original.getMessage()); } - @Ignore("This currently doesn't fail properly - should it?") + @AwaitsFix(bugUrl = "TODO") public void testCannotReferToRuntimeFields() throws IOException { DocumentMapper mapper = createDocumentMapper(topMapping(b -> { b.startObject("runtime"); From 4b35f37cb9ea967025c0fdc6395ba4f7931b3ce9 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 15 Mar 2021 12:35:23 +0000 Subject: [PATCH 09/39] split postparsecontext into context and phase executor --- .../index/mapper/DocumentParser.java | 2 +- .../index/mapper/MappingLookup.java | 2 +- .../index/mapper/PostParseContext.java | 233 +--------------- .../index/mapper/PostParsePhase.java | 264 ++++++++++++++++++ 4 files changed, 268 insertions(+), 233 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index d787133018900..c71985e9065b8 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -108,7 +108,7 @@ private static void internalParseDocument(MappingLookup lookup, MetadataFieldMap parseObjectOrNested(context, root); } - PostParseContext.executePostParsePhases(lookup, context); + PostParsePhase.executePostParsePhases(lookup, context); for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) { metadataMapper.postParse(context); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java index 88588280bc6f0..e02371afa551c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -179,7 +179,7 @@ public NamedAnalyzer indexAnalyzer(String field, Function return unmappedFieldAnalyzer.apply(field); } - public Map getPostParsePhases() { + public Map getPostParseExecutors() { return postParsePhases; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java index f259efb75447b..eab8069f321bc 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java @@ -8,53 +8,18 @@ package org.elasticsearch.index.mapper; -import org.apache.lucene.analysis.Analyzer; -import org.apache.lucene.index.BinaryDocValues; -import org.apache.lucene.index.DocValuesType; -import org.apache.lucene.index.FieldInfo; -import org.apache.lucene.index.FieldInfos; -import org.apache.lucene.index.Fields; -import org.apache.lucene.index.IndexOptions; -import org.apache.lucene.index.IndexableField; -import org.apache.lucene.index.LeafMetaData; -import org.apache.lucene.index.LeafReader; import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.index.NumericDocValues; -import org.apache.lucene.index.PointValues; -import org.apache.lucene.index.SortedDocValues; -import org.apache.lucene.index.SortedNumericDocValues; -import org.apache.lucene.index.SortedSetDocValues; -import org.apache.lucene.index.StoredFieldVisitor; -import org.apache.lucene.index.Terms; -import org.apache.lucene.index.memory.MemoryIndex; -import org.apache.lucene.util.Bits; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.index.fielddata.IndexFieldDataCache; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.search.lookup.SearchLookup; -import java.io.IOException; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; - public class PostParseContext { public final SearchLookup searchLookup; public final LeafReaderContext leafReaderContext; public final ParseContext pc; - private final Map fieldExecutors = new HashMap<>(); - - public static void executePostParsePhases(MappingLookup lookup, ParseContext parseContext) throws IOException { - if (lookup.getPostParsePhases().isEmpty()) { - return; - } - PostParseContext context = new PostParseContext(lookup, parseContext); - context.executePostParse(); - } - private PostParseContext(MappingLookup mappingLookup, ParseContext pc) { + public PostParseContext(MappingLookup mappingLookup, ParseContext pc, LeafReaderContext ctx) { this.searchLookup = new SearchLookup( mappingLookup::getFieldType, (ft, s) -> ft.fielddataBuilder(pc.indexSettings().getIndex().getName(), s).build( @@ -62,201 +27,7 @@ private PostParseContext(MappingLookup mappingLookup, ParseContext pc) { new NoneCircuitBreakerService()) ); this.pc = pc; - LazyDocumentReader reader = new LazyDocumentReader( - pc.rootDoc(), - pc.sourceToParse().source(), - mappingLookup.getPostParsePhases().keySet()); - this.leafReaderContext = reader.getContext(); - mappingLookup.getPostParsePhases().forEach((k, c) -> fieldExecutors.put(k, new FieldExecutor(c))); - } - - private void executePostParse() throws IOException { - for (FieldExecutor ms : fieldExecutors.values()) { - ms.execute(); - } - } - - private void executeField(String field) throws IOException { - assert fieldExecutors.containsKey(field); - fieldExecutors.get(field).execute(); - } - - private class FieldExecutor { - PostParseExecutor executor; - boolean executed = false; - FieldExecutor(PostParseExecutor executor) { - this.executor = executor; - } - - void execute() throws IOException { - if (executed == false) { - executor.execute(PostParseContext.this); - executed = true; - } - } + this.leafReaderContext = ctx; } - private class LazyDocumentReader extends LeafReader { - - private final ParseContext.Document document; - private final BytesReference sourceBytes; - private final Set calculatedFields; - private LeafReaderContext in; - - private LazyDocumentReader(ParseContext.Document document, BytesReference sourceBytes, Set calculatedFields) { - this.document = document; - this.sourceBytes = sourceBytes; - this.calculatedFields = calculatedFields; - } - - private void buildDocValues(String field) throws IOException { - if (calculatedFields.contains(field)) { - // this means that a mapper script is referring to another calculated field; - // in which case we need to execute that field first, and then rebuild the - // memory index - executeField(field); - calculatedFields.remove(field); - this.in = null; - } - if (in != null) { - return; - } - MemoryIndex mi = new MemoryIndex(); - for (IndexableField f : document) { - if (f.fieldType().docValuesType() != null) { - mi.addField(f, EMPTY_ANALYZER); - } - } - mi.freeze(); - this.in = mi.createSearcher().getIndexReader().leaves().get(0); - } - - @Override - public NumericDocValues getNumericDocValues(String field) throws IOException { - buildDocValues(field); - return in.reader().getNumericDocValues(field); - } - - @Override - public BinaryDocValues getBinaryDocValues(String field) throws IOException { - buildDocValues(field); - return in.reader().getBinaryDocValues(field); - } - - @Override - public SortedDocValues getSortedDocValues(String field) throws IOException { - buildDocValues(field); - return in.reader().getSortedDocValues(field); - } - - @Override - public SortedNumericDocValues getSortedNumericDocValues(String field) throws IOException { - buildDocValues(field); - return in.reader().getSortedNumericDocValues(field); - } - - @Override - public SortedSetDocValues getSortedSetDocValues(String field) throws IOException { - buildDocValues(field); - return in.reader().getSortedSetDocValues(field); - } - - @Override - public FieldInfos getFieldInfos() { - try { - buildDocValues(null); - } catch (IOException e) { - // won't actually happen - } - return in.reader().getFieldInfos(); - } - - @Override - public void document(int docID, StoredFieldVisitor visitor) throws IOException { - visitor.binaryField(SOURCE_FIELD_INFO, sourceBytes.toBytesRef().bytes); - } - - @Override - public CacheHelper getCoreCacheHelper() { - throw new UnsupportedOperationException(); - } - - @Override - public Terms terms(String field) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public NumericDocValues getNormValues(String field) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public Bits getLiveDocs() { - throw new UnsupportedOperationException(); - } - - @Override - public PointValues getPointValues(String field) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public void checkIntegrity() throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public LeafMetaData getMetaData() { - throw new UnsupportedOperationException(); - } - - @Override - public Fields getTermVectors(int docID) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public int numDocs() { - throw new UnsupportedOperationException(); - } - - @Override - public int maxDoc() { - throw new UnsupportedOperationException(); - } - - @Override - protected void doClose() throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public CacheHelper getReaderCacheHelper() { - throw new UnsupportedOperationException(); - } - } - - private static final FieldInfo SOURCE_FIELD_INFO = new FieldInfo( - "_source", - 0, - false, - false, - false, - IndexOptions.NONE, - DocValuesType.NONE, - -1, - Collections.emptyMap(), - 0, - 0, - 0, - false - ); - - private static final Analyzer EMPTY_ANALYZER = new Analyzer() { - @Override - protected TokenStreamComponents createComponents(String fieldName) { - return new TokenStreamComponents(reader -> {}, null); - } - }; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java new file mode 100644 index 0000000000000..68c1eefaefffe --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java @@ -0,0 +1,264 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.index.mapper; + +import org.apache.lucene.analysis.Analyzer; +import org.apache.lucene.index.BinaryDocValues; +import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.index.FieldInfo; +import org.apache.lucene.index.FieldInfos; +import org.apache.lucene.index.Fields; +import org.apache.lucene.index.IndexOptions; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.index.LeafMetaData; +import org.apache.lucene.index.LeafReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.PointValues; +import org.apache.lucene.index.SortedDocValues; +import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.index.SortedSetDocValues; +import org.apache.lucene.index.StoredFieldVisitor; +import org.apache.lucene.index.Terms; +import org.apache.lucene.index.memory.MemoryIndex; +import org.apache.lucene.util.Bits; +import org.elasticsearch.common.bytes.BytesReference; + +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +/** + * Executes post parse phases on mappings + */ +public class PostParsePhase { + + private final Map fieldExecutors = new HashMap<>(); + private final PostParseContext context; + + /** + * Given a mapping, collects all {@link PostParseExecutor}s and executes them + * @param lookup the MappingLookup to collect executors from + * @param parseContext the ParseContext of the current document + * @throws IOException + */ + public static void executePostParsePhases(MappingLookup lookup, ParseContext parseContext) throws IOException { + if (lookup.getPostParseExecutors().isEmpty()) { + return; + } + PostParsePhase postParsePhase = new PostParsePhase(lookup, parseContext); + postParsePhase.executePostParse(); + } + + private PostParsePhase(MappingLookup mappingLookup, ParseContext pc) { + LazyDocumentReader reader = new LazyDocumentReader( + pc.rootDoc(), + pc.sourceToParse().source(), + mappingLookup.getPostParseExecutors().keySet()); + this.context = new PostParseContext(mappingLookup, pc, reader.getContext()); + mappingLookup.getPostParseExecutors().forEach((k, c) -> fieldExecutors.put(k, new OneTimeFieldExecutor(c))); + } + + private void executePostParse() throws IOException { + for (OneTimeFieldExecutor ms : fieldExecutors.values()) { + ms.execute(); + } + } + + private void executeField(String field) throws IOException { + assert fieldExecutors.containsKey(field); + fieldExecutors.get(field).execute(); + } + + // FieldExecutors can be called both by executePostParse() and from the lazy reader, + // so to ensure that we don't run field scripts multiple times we guard them with + // this one-time executor class + private class OneTimeFieldExecutor { + + PostParseExecutor executor; + boolean executed = false; + + OneTimeFieldExecutor(PostParseExecutor executor) { + this.executor = executor; + } + + void execute() throws IOException { + if (executed == false) { + executor.execute(context); + executed = true; + } + } + } + + private class LazyDocumentReader extends LeafReader { + + private final ParseContext.Document document; + private final BytesReference sourceBytes; + private final Set calculatedFields; + private LeafReaderContext in; + + private LazyDocumentReader(ParseContext.Document document, BytesReference sourceBytes, Set calculatedFields) { + this.document = document; + this.sourceBytes = sourceBytes; + this.calculatedFields = calculatedFields; + } + + private void buildDocValues(String field) throws IOException { + if (calculatedFields.contains(field)) { + // this means that a mapper script is referring to another calculated field; + // in which case we need to execute that field first, and then rebuild the + // memory index + executeField(field); + calculatedFields.remove(field); + this.in = null; + } + if (in != null) { + return; + } + MemoryIndex mi = new MemoryIndex(); + for (IndexableField f : document) { + if (f.fieldType().docValuesType() != null) { + mi.addField(f, EMPTY_ANALYZER); + } + } + mi.freeze(); + this.in = mi.createSearcher().getIndexReader().leaves().get(0); + } + + @Override + public NumericDocValues getNumericDocValues(String field) throws IOException { + buildDocValues(field); + return in.reader().getNumericDocValues(field); + } + + @Override + public BinaryDocValues getBinaryDocValues(String field) throws IOException { + buildDocValues(field); + return in.reader().getBinaryDocValues(field); + } + + @Override + public SortedDocValues getSortedDocValues(String field) throws IOException { + buildDocValues(field); + return in.reader().getSortedDocValues(field); + } + + @Override + public SortedNumericDocValues getSortedNumericDocValues(String field) throws IOException { + buildDocValues(field); + return in.reader().getSortedNumericDocValues(field); + } + + @Override + public SortedSetDocValues getSortedSetDocValues(String field) throws IOException { + buildDocValues(field); + return in.reader().getSortedSetDocValues(field); + } + + @Override + public FieldInfos getFieldInfos() { + try { + buildDocValues(null); + } catch (IOException e) { + // won't actually happen + } + return in.reader().getFieldInfos(); + } + + @Override + public void document(int docID, StoredFieldVisitor visitor) throws IOException { + visitor.binaryField(SOURCE_FIELD_INFO, sourceBytes.toBytesRef().bytes); + } + + @Override + public CacheHelper getCoreCacheHelper() { + throw new UnsupportedOperationException(); + } + + @Override + public Terms terms(String field) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public NumericDocValues getNormValues(String field) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public Bits getLiveDocs() { + throw new UnsupportedOperationException(); + } + + @Override + public PointValues getPointValues(String field) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public void checkIntegrity() throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public LeafMetaData getMetaData() { + throw new UnsupportedOperationException(); + } + + @Override + public Fields getTermVectors(int docID) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public int numDocs() { + throw new UnsupportedOperationException(); + } + + @Override + public int maxDoc() { + throw new UnsupportedOperationException(); + } + + @Override + protected void doClose() throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public CacheHelper getReaderCacheHelper() { + throw new UnsupportedOperationException(); + } + } + + private static final FieldInfo SOURCE_FIELD_INFO = new FieldInfo( + "_source", + 0, + false, + false, + false, + IndexOptions.NONE, + DocValuesType.NONE, + -1, + Collections.emptyMap(), + 0, + 0, + 0, + false + ); + + private static final Analyzer EMPTY_ANALYZER = new Analyzer() { + @Override + protected TokenStreamComponents createComponents(String fieldName) { + return new TokenStreamComponents(reader -> {}, null); + } + }; +} From 4d79be2738c9059221931637ba5cae3557da7643 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 15 Mar 2021 14:13:46 +0000 Subject: [PATCH 10/39] Make defining test scripts easier --- .../index/mapper/CalculatedFieldTests.java | 163 ++++++++++++------ 1 file changed, 111 insertions(+), 52 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index 29e6da6bb172f..b84d73b487c7a 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -9,6 +9,7 @@ package org.elasticsearch.index.mapper; import org.apache.lucene.index.IndexableField; +import org.apache.lucene.index.LeafReaderContext; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.Plugin; @@ -17,12 +18,15 @@ import org.elasticsearch.runtimefields.mapper.LongFieldScript; import org.elasticsearch.script.ScriptContext; import org.elasticsearch.script.ScriptEngine; +import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Consumer; public class CalculatedFieldTests extends MapperServiceTestCase { @@ -36,7 +40,7 @@ public void testCalculatedFieldLength() throws IOException { b.startObject("message").field("type", "text").endObject(); b.startObject("message_length"); b.field("type", "long"); - b.field("script", "length"); + b.field("script", "message_length"); b.endObject(); })); @@ -52,7 +56,7 @@ public void testDocAccess() throws IOException { b.startObject("long_field").field("type", "long").endObject(); b.startObject("long_field_plus_two"); b.field("type", "long"); - b.field("script", "plus_two"); + b.field("script", "long_field_plus_two"); b.endObject(); })); @@ -65,7 +69,7 @@ public void testDoublesAccess() throws IOException { b.startObject("double_field").field("type", "double").endObject(); b.startObject("double_field_plus_two"); b.field("type", "double"); - b.field("script", "plus_two_double_field"); + b.field("script", "double_field_plus_two"); b.endObject(); })); @@ -78,12 +82,12 @@ public void testSerialization() throws IOException { b.startObject("message").field("type", "text").endObject(); b.startObject("message_length"); b.field("type", "long"); - b.field("script", "length"); + b.field("script", "message_length"); b.endObject(); })); assertEquals( "{\"_doc\":{\"properties\":{\"message\":{\"type\":\"text\"}," + - "\"message_length\":{\"type\":\"long\",\"script\":{\"source\":\"length\",\"lang\":\"painless\"}}}}}", + "\"message_length\":{\"type\":\"long\",\"script\":{\"source\":\"message_length\",\"lang\":\"painless\"}}}}}", Strings.toString(mapper.mapping())); } @@ -96,11 +100,11 @@ public void testCrossReferences() throws IOException { b.endObject(); b.startObject("message_length"); b.field("type", "long"); - b.field("script", "length"); + b.field("script", "message_length"); b.endObject(); b.startObject("message_length_plus_four"); b.field("type", "double"); - b.field("script", "plus_two_message_length_plus_two"); + b.field("script", "message_length_plus_two_plus_two"); b.endObject(); })); ParsedDocument doc = mapper.parse(source(b -> b.field("message", "this is a message"))); @@ -135,7 +139,7 @@ public void testCannotReferToRuntimeFields() throws IOException { b.startObject("runtime-field").field("type", "long").endObject(); b.endObject(); b.startObject("properties"); - b.startObject("index-field").field("type", "long").field("script", "refer_to_runtime").endObject(); + b.startObject("index-field").field("type", "long").field("script", "refer-to-runtime").endObject(); b.endObject(); })); @@ -143,6 +147,99 @@ public void testCannotReferToRuntimeFields() throws IOException { assertEquals("Cannot reference runtime field [runtime-field] in an index-time script", e.getMessage()); } + private static Consumer getLongScript(String name) { + if ("refer-to-runtime".equals(name)) { + return s -> { + s.emitValue((long) s.getDoc().get("runtime-field").get(0)); + }; + } + if (name.endsWith("_length")) { + String field = name.substring(0, name.lastIndexOf("_length")); + return s -> { + for (Object v : s.extractValuesFromSource(field)) { + s.emitValue(Objects.toString(v).length()); + } + }; + } + if (name.endsWith("_plus_two")) { + String field = name.substring(0, name.lastIndexOf("_plus_two")); + return s -> { + long input = (long) s.getDoc().get(field).get(0); + s.emitValue(input + 2); + }; + } + throw new UnsupportedOperationException("Unknown script [" + name + "]"); + } + + private static Consumer getDoubleScript(String name) { + if (name.endsWith("_plus_two")) { + String field = name.substring(0, name.lastIndexOf("_plus_two")); + return s -> { + Number input = (Number) s.getDoc().get(field).get(0); + s.emitValue(input.doubleValue() + 2); + }; + } + throw new UnsupportedOperationException("Unknown script [" + name + "]"); + } + + private static class TestLongFieldScript extends LongFieldScript { + + final Consumer executor; + + TestLongFieldScript( + String fieldName, + Map params, + SearchLookup searchLookup, + LeafReaderContext ctx, + Consumer executor + ) { + super(fieldName, params, searchLookup, ctx); + this.executor = executor; + } + + @Override + public void execute() { + executor.accept(this); + } + + public void emitValue(long v) { + super.emit(v); + } + + public List extractValuesFromSource(String path) { + return super.extractFromSource(path); + } + } + + private static class TestDoubleFieldScript extends DoubleFieldScript { + + final Consumer executor; + + TestDoubleFieldScript( + String fieldName, + Map params, + SearchLookup searchLookup, + LeafReaderContext ctx, + Consumer executor + ) { + super(fieldName, params, searchLookup, ctx); + this.executor = executor; + } + + @Override + public void execute() { + executor.accept(this); + } + + public void emitValue(double v) { + super.emit(v); + } + + public List extractValuesFromSource(String path) { + return super.extractFromSource(path); + } + } + public static class TestScriptPlugin extends Plugin implements ScriptPlugin { @Override @@ -162,52 +259,14 @@ public FactoryType compile( Map params ) { if (context.factoryClazz == LongFieldScript.Factory.class) { - switch (code) { - case "length": - return (FactoryType) (LongFieldScript.Factory) (n, p, l) -> ctx -> new LongFieldScript(n, p, l, ctx) { - @Override - public void execute() { - for (Object v : extractFromSource("message")) { - emit(Objects.toString(v).length()); - } - } - }; - case "plus_two": - return (FactoryType) (LongFieldScript.Factory) (n, p, l) -> ctx -> new LongFieldScript(n, p, l, ctx) { - @Override - public void execute() { - long input = (long) getDoc().get("long_field").get(0); - emit(input + 2); - } - }; - case "message_length_plus_two": - return (FactoryType) (LongFieldScript.Factory) (n, p, l) -> ctx -> new LongFieldScript(n, p, l, ctx) { - @Override - public void execute() { - long input = (long) getDoc().get("message_length").get(0); - emit(input + 2); - } - }; - case "refer_to_runtime": - return (FactoryType) (LongFieldScript.Factory) (n, p, l) -> ctx -> new LongFieldScript(n, p, l, ctx) { - @Override - public void execute() { - emit((long) getDoc().get("runtime-field").get(0)); - } - }; - } + return (FactoryType) (LongFieldScript.Factory) (n, p, l) -> ctx -> new TestLongFieldScript( + n, p, l, ctx, getLongScript(name) + ); } if (context.factoryClazz == DoubleFieldScript.Factory.class) { - if (code.startsWith("plus_two_")) { - String sourceField = code.substring(9); - return (FactoryType) (DoubleFieldScript.Factory) (n, p, l) -> ctx -> new DoubleFieldScript(n, p, l, ctx) { - @Override - public void execute() { - double input = ((Number) getDoc().get(sourceField).get(0)).doubleValue(); - emit(input + 2); - } - }; - } + return (FactoryType) (DoubleFieldScript.Factory) (n, p, l) -> ctx -> new TestDoubleFieldScript( + n, p, l, ctx, getDoubleScript(name) + ); } throw new IllegalArgumentException("Unknown factory type " + context.factoryClazz + " for code " + code); } From cead31664b9e97fc39cdde51292c9dc3cedc3cc7 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 15 Mar 2021 15:53:23 +0000 Subject: [PATCH 11/39] loop detection --- .../index/mapper/PostParsePhase.java | 8 +++++++- .../index/mapper/CalculatedFieldTests.java | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java index 68c1eefaefffe..517f455acd589 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java @@ -33,6 +33,7 @@ import java.io.IOException; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; @@ -103,6 +104,7 @@ private class LazyDocumentReader extends LeafReader { private final ParseContext.Document document; private final BytesReference sourceBytes; private final Set calculatedFields; + private final Set fieldPath = new LinkedHashSet<>(); private LeafReaderContext in; private LazyDocumentReader(ParseContext.Document document, BytesReference sourceBytes, Set calculatedFields) { @@ -115,9 +117,13 @@ private void buildDocValues(String field) throws IOException { if (calculatedFields.contains(field)) { // this means that a mapper script is referring to another calculated field; // in which case we need to execute that field first, and then rebuild the - // memory index + // memory index. We also check for loops here + if (fieldPath.add(field) == false) { + throw new IllegalStateException("Loop in field resolution detected: " + String.join("->", fieldPath) + "->" + field); + } executeField(field); calculatedFields.remove(field); + fieldPath.remove(field); this.in = null; } if (in != null) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index b84d73b487c7a..4f4eaa7351c58 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -28,6 +28,8 @@ import java.util.Set; import java.util.function.Consumer; +import static org.hamcrest.Matchers.containsString; + public class CalculatedFieldTests extends MapperServiceTestCase { @Override @@ -132,6 +134,20 @@ public void testCannotIndexDirectlyIntoScriptMapper() throws IOException { assertEquals("Cannot index data directly into scripted field", original.getMessage()); } + public void testLoopDetection() throws IOException { + DocumentMapper mapper = createDocumentMapper(mapping(b -> { + b.startObject("field1").field("type", "long").field("script", "field2_plus_two").endObject(); + b.startObject("field2").field("type", "long").field("script", "field1_plus_two").endObject(); + })); + + Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> {}))); + assertEquals("failed to parse", e.getMessage()); + assertThat(e.getCause().getMessage(), containsString("Loop in field resolution detected")); + // Can be either field1->field2->field1 or field2->field1->field2 because + // post-phase executor order is not deterministic + assertThat(e.getCause().getMessage(), containsString("field1->field2")); + } + @AwaitsFix(bugUrl = "TODO") public void testCannotReferToRuntimeFields() throws IOException { DocumentMapper mapper = createDocumentMapper(topMapping(b -> { From 1faea30283ba2cc6ff915c923765de5046bd4263 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 15 Mar 2021 16:29:36 +0000 Subject: [PATCH 12/39] disallow stored scripts --- .../org/elasticsearch/index/mapper/ScriptParams.java | 4 ++++ .../elasticsearch/index/mapper/CalculatedFieldTests.java | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java b/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java index efeda8d9ec8f9..afbd1b70b55b4 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java @@ -10,6 +10,7 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.ScriptType; import java.util.function.BiFunction; import java.util.function.Function; @@ -41,6 +42,9 @@ public static FieldMapper.Parameter> script( return null; } Script script = Script.parse(o); + if (script.getType() == ScriptType.STORED) { + throw new IllegalArgumentException("stored scripts are not supported on scripted field [" + n + "]"); + } return compiler.apply(script, c.scriptService()); }, initializer diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index 4f4eaa7351c58..a41b4e6e5316a 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -29,6 +29,7 @@ import java.util.function.Consumer; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; public class CalculatedFieldTests extends MapperServiceTestCase { @@ -148,6 +149,14 @@ public void testLoopDetection() throws IOException { assertThat(e.getCause().getMessage(), containsString("field1->field2")); } + public void testStoredScriptsNotPermitted() { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + b.field("type", "long"); + b.startObject("script").field("id", "foo").endObject(); + }))); + assertThat(e.getMessage(), equalTo("Failed to parse mapping: stored scripts are not supported on scripted field [field]")); + } + @AwaitsFix(bugUrl = "TODO") public void testCannotReferToRuntimeFields() throws IOException { DocumentMapper mapper = createDocumentMapper(topMapping(b -> { From 3b26fe3e6fb9a5ef0a6fe3e3c06d02ba22f72096 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 15 Mar 2021 17:45:38 +0000 Subject: [PATCH 13/39] Don't use memory index --- .../index/mapper/PostParsePhase.java | 297 +++++++++++++++--- 1 file changed, 259 insertions(+), 38 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java index 517f455acd589..76666f89096c3 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java @@ -8,7 +8,6 @@ package org.elasticsearch.index.mapper; -import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.index.BinaryDocValues; import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.FieldInfo; @@ -18,7 +17,6 @@ import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.LeafMetaData; import org.apache.lucene.index.LeafReader; -import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.NumericDocValues; import org.apache.lucene.index.PointValues; import org.apache.lucene.index.SortedDocValues; @@ -26,16 +24,19 @@ import org.apache.lucene.index.SortedSetDocValues; import org.apache.lucene.index.StoredFieldVisitor; import org.apache.lucene.index.Terms; -import org.apache.lucene.index.memory.MemoryIndex; import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.bytes.BytesReference; import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; /** * Executes post parse phases on mappings @@ -105,7 +106,6 @@ private class LazyDocumentReader extends LeafReader { private final BytesReference sourceBytes; private final Set calculatedFields; private final Set fieldPath = new LinkedHashSet<>(); - private LeafReaderContext in; private LazyDocumentReader(ParseContext.Document document, BytesReference sourceBytes, Set calculatedFields) { this.document = document; @@ -113,7 +113,7 @@ private LazyDocumentReader(ParseContext.Document document, BytesReference source this.calculatedFields = calculatedFields; } - private void buildDocValues(String field) throws IOException { + private void checkField(String field) throws IOException { if (calculatedFields.contains(field)) { // this means that a mapper script is referring to another calculated field; // in which case we need to execute that field first, and then rebuild the @@ -124,59 +124,71 @@ private void buildDocValues(String field) throws IOException { executeField(field); calculatedFields.remove(field); fieldPath.remove(field); - this.in = null; } - if (in != null) { - return; - } - MemoryIndex mi = new MemoryIndex(); - for (IndexableField f : document) { - if (f.fieldType().docValuesType() != null) { - mi.addField(f, EMPTY_ANALYZER); - } - } - mi.freeze(); - this.in = mi.createSearcher().getIndexReader().leaves().get(0); } @Override public NumericDocValues getNumericDocValues(String field) throws IOException { - buildDocValues(field); - return in.reader().getNumericDocValues(field); + checkField(field); + List values = document.getFields().stream() + .filter(f -> Objects.equals(f.name(), field)) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.NUMERIC) + .map(IndexableField::numericValue) + .sorted() + .collect(Collectors.toList()); + return numericDocValues(values); } @Override public BinaryDocValues getBinaryDocValues(String field) throws IOException { - buildDocValues(field); - return in.reader().getBinaryDocValues(field); + checkField(field); + List values = document.getFields().stream() + .filter(f -> Objects.equals(f.name(), field)) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.BINARY) + .map(IndexableField::binaryValue) + .sorted() + .collect(Collectors.toList()); + return binaryDocValues(values); } @Override public SortedDocValues getSortedDocValues(String field) throws IOException { - buildDocValues(field); - return in.reader().getSortedDocValues(field); + checkField(field); + List values = document.getFields().stream() + .filter(f -> Objects.equals(f.name(), field)) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.BINARY) + .map(IndexableField::binaryValue) + .sorted() + .collect(Collectors.toList()); + return sortedDocValues(values); } @Override public SortedNumericDocValues getSortedNumericDocValues(String field) throws IOException { - buildDocValues(field); - return in.reader().getSortedNumericDocValues(field); + checkField(field); + List values = document.getFields().stream() + .filter(f -> Objects.equals(f.name(), field)) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.SORTED_NUMERIC) + .map(IndexableField::numericValue) + .sorted() + .collect(Collectors.toList()); + return sortedNumericDocValues(values); } @Override public SortedSetDocValues getSortedSetDocValues(String field) throws IOException { - buildDocValues(field); - return in.reader().getSortedSetDocValues(field); + List values = document.getFields().stream() + .filter(f -> Objects.equals(f.name(), field)) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.BINARY) + .map(IndexableField::binaryValue) + .sorted() + .collect(Collectors.toList()); + return sortedSetDocValues(values); } @Override public FieldInfos getFieldInfos() { - try { - buildDocValues(null); - } catch (IOException e) { - // won't actually happen - } - return in.reader().getFieldInfos(); + throw new UnsupportedOperationException(); } @Override @@ -261,10 +273,219 @@ public CacheHelper getReaderCacheHelper() { false ); - private static final Analyzer EMPTY_ANALYZER = new Analyzer() { - @Override - protected TokenStreamComponents createComponents(String fieldName) { - return new TokenStreamComponents(reader -> {}, null); + private static NumericDocValues numericDocValues(List values) { + if (values.size() == 0) { + return null; + } + return new NumericDocValues() { + @Override + public long longValue() { + return values.get(0).longValue(); + } + + @Override + public boolean advanceExact(int target) { + return true; + } + + @Override + public int docID() { + return 0; + } + + @Override + public int nextDoc() { + return 0; + } + + @Override + public int advance(int target) { + return 0; + } + + @Override + public long cost() { + return 0; + } + }; + } + + private static SortedNumericDocValues sortedNumericDocValues(List values) { + return new SortedNumericDocValues() { + + int i = -1; + + @Override + public long nextValue() { + i++; + return values.get(i).longValue(); + } + + @Override + public int docValueCount() { + return values.size(); + } + + @Override + public boolean advanceExact(int target) { + return true; + } + + @Override + public int docID() { + return 0; + } + + @Override + public int nextDoc() { + return 0; + } + + @Override + public int advance(int target) { + return 0; + } + + @Override + public long cost() { + return 0; + } + }; + } + + private static BinaryDocValues binaryDocValues(List values) { + if (values.size() == 0) { + return null; + } + return new BinaryDocValues() { + @Override + public BytesRef binaryValue() { + return values.get(0); + } + + @Override + public boolean advanceExact(int target) { + return false; + } + + @Override + public int docID() { + return 0; + } + + @Override + public int nextDoc() { + return 0; + } + + @Override + public int advance(int target) { + return 0; + } + + @Override + public long cost() { + return 0; + } + }; + } + + private static SortedDocValues sortedDocValues(List values) { + if (values.size() == 0) { + return null; + } + return new SortedDocValues() { + + @Override + public int ordValue() { + return 0; + } + + @Override + public BytesRef lookupOrd(int ord) { + return values.get(0); + } + + @Override + public int getValueCount() { + return values.size(); + } + + @Override + public boolean advanceExact(int target) { + return true; + } + + @Override + public int docID() { + return 0; + } + + @Override + public int nextDoc() { + return 0; + } + + @Override + public int advance(int target) { + return 0; + } + + @Override + public long cost() { + return 0; + } + }; + } + + private static SortedSetDocValues sortedSetDocValues(List values) { + if (values.size() == 0) { + return null; } - }; + return new SortedSetDocValues() { + + int i = -1; + + @Override + public long nextOrd() { + i++; + return i; + } + + @Override + public BytesRef lookupOrd(long ord) { + return values.get((int)ord); + } + + @Override + public long getValueCount() { + return values.size(); + } + + @Override + public boolean advanceExact(int target) { + return true; + } + + @Override + public int docID() { + return 0; + } + + @Override + public int nextDoc() { + return 0; + } + + @Override + public int advance(int target) { + return 0; + } + + @Override + public long cost() { + return 0; + } + }; + } } From 2e2dde37d1c4f96ede1a1aa0eca432a0a6cdb714 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 15 Mar 2021 17:53:36 +0000 Subject: [PATCH 14/39] tidy --- .../index/mapper/PostParseExecutor.java | 4 +-- .../index/mapper/PostParsePhase.java | 31 +++++++------------ 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java index a9e40aa4dc51c..0cd7477e24ebf 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java @@ -8,10 +8,8 @@ package org.elasticsearch.index.mapper; -import java.io.IOException; - public interface PostParseExecutor { - void execute(PostParseContext context) throws IOException; + void execute(PostParseContext context); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java index 76666f89096c3..c566d5adbf674 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java @@ -50,14 +50,15 @@ public class PostParsePhase { * Given a mapping, collects all {@link PostParseExecutor}s and executes them * @param lookup the MappingLookup to collect executors from * @param parseContext the ParseContext of the current document - * @throws IOException */ - public static void executePostParsePhases(MappingLookup lookup, ParseContext parseContext) throws IOException { + public static void executePostParsePhases(MappingLookup lookup, ParseContext parseContext) { if (lookup.getPostParseExecutors().isEmpty()) { return; } PostParsePhase postParsePhase = new PostParsePhase(lookup, parseContext); - postParsePhase.executePostParse(); + for (OneTimeFieldExecutor executor : postParsePhase.fieldExecutors.values()) { + executor.execute(); + } } private PostParsePhase(MappingLookup mappingLookup, ParseContext pc) { @@ -69,17 +70,6 @@ private PostParsePhase(MappingLookup mappingLookup, ParseContext pc) { mappingLookup.getPostParseExecutors().forEach((k, c) -> fieldExecutors.put(k, new OneTimeFieldExecutor(c))); } - private void executePostParse() throws IOException { - for (OneTimeFieldExecutor ms : fieldExecutors.values()) { - ms.execute(); - } - } - - private void executeField(String field) throws IOException { - assert fieldExecutors.containsKey(field); - fieldExecutors.get(field).execute(); - } - // FieldExecutors can be called both by executePostParse() and from the lazy reader, // so to ensure that we don't run field scripts multiple times we guard them with // this one-time executor class @@ -92,7 +82,7 @@ private class OneTimeFieldExecutor { this.executor = executor; } - void execute() throws IOException { + void execute() { if (executed == false) { executor.execute(context); executed = true; @@ -113,15 +103,15 @@ private LazyDocumentReader(ParseContext.Document document, BytesReference source this.calculatedFields = calculatedFields; } - private void checkField(String field) throws IOException { + private void checkField(String field) { if (calculatedFields.contains(field)) { // this means that a mapper script is referring to another calculated field; - // in which case we need to execute that field first, and then rebuild the - // memory index. We also check for loops here + // in which case we need to execute that field first. We also check for loops here if (fieldPath.add(field) == false) { throw new IllegalStateException("Loop in field resolution detected: " + String.join("->", fieldPath) + "->" + field); } - executeField(field); + assert fieldExecutors.containsKey(field); + fieldExecutors.get(field).execute(); calculatedFields.remove(field); fieldPath.remove(field); } @@ -311,6 +301,9 @@ public long cost() { } private static SortedNumericDocValues sortedNumericDocValues(List values) { + if (values.size() == 0) { + return null; + } return new SortedNumericDocValues() { int i = -1; From 2e017777385fa65ff0f6ea3c3c054faeb48d6f31 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 16 Mar 2021 10:09:16 +0000 Subject: [PATCH 15/39] cleanup --- .../index/mapper/FieldMapper.java | 32 ++++++ .../index/mapper/NumberFieldMapper.java | 102 ++++++------------ .../index/mapper/ObjectMapper.java | 1 - .../index/mapper/ScriptParams.java | 53 --------- 4 files changed, 65 insertions(+), 123 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index 9c757fa005c63..1417d0d36f20a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -24,6 +24,9 @@ import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.mapper.FieldNamesFieldMapper.FieldNamesFieldType; import org.elasticsearch.index.mapper.Mapper.TypeParser.ParserContext; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptService; +import org.elasticsearch.script.ScriptType; import java.io.IOException; import java.util.ArrayList; @@ -830,6 +833,35 @@ public static Parameter docValuesParam(Function i return Parameter.boolParam("doc_values", false, initializer, defaultValue); } + /** + * Defines a script parameter + * @param compiler a function that produces a compiled script, given a {@link Script} and {@link ScriptService} + * @param initializer retrieves the equivalent parameter from an existing FieldMapper for use in merges + * @param the type of the compiled script + * @return a script parameter + */ + public static FieldMapper.Parameter> scriptParam( + BiFunction> compiler, + Function>initializer + ) { + return new FieldMapper.Parameter<>( + "script", + false, + () -> null, + (n, c, o) -> { + if (o == null) { + return null; + } + Script script = Script.parse(o); + if (script.getType() == ScriptType.STORED) { + throw new IllegalArgumentException("stored scripts are not supported on scripted field [" + n + "]"); + } + return compiler.apply(script, c.scriptService()); + }, + initializer + ).acceptsNull(); + } + } public static final class Conflicts { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 7ce527dffa856..5e6ababe4eeb7 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -51,7 +51,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -106,7 +105,7 @@ public Builder(String name, NumberType type, boolean ignoreMalformedByDefault, b = Parameter.explicitBoolParam("coerce", true, m -> toType(m).coerce, coerceByDefault); this.nullValue = new Parameter<>("null_value", false, () -> null, (n, c, o) -> o == null ? null : type.parse(o, false), m -> toType(m).nullValue).acceptsNull(); - this.script = ScriptParams.script( + this.script = Parameter.scriptParam( type.compiler(name), m -> toType(m).script ); @@ -365,8 +364,22 @@ public Double parse(XContentParser parser, boolean coerce) throws IOException { @Override public BiFunction> compiler(String fieldName) { - return (script, service) - -> new DoubleMapperScript(fieldName, script, service.compile(script, DoubleFieldScript.CONTEXT), script.getParams()); + return (script, service) -> new MapperScript<>(script) { + + final DoubleFieldScript.Factory scriptFactory = service.compile(script, DoubleFieldScript.CONTEXT); + + @Override + public void executeAndEmit(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { + DoubleFieldScript s = scriptFactory + .newFactory(fieldName, script.getParams(), lookup) + .newInstance(ctx); + s.runForDoc(doc); + double[] vs = s.values(); + for (int i = 0; i < s.count(); i++) { + emitter.accept(vs[i]); + } + } + }; } @Override @@ -695,8 +708,22 @@ public Long parse(XContentParser parser, boolean coerce) throws IOException { @Override public BiFunction> compiler(String fieldName) { - return (script, service) - -> new LongMapperScript(fieldName, script, service.compile(script, LongFieldScript.CONTEXT), script.getParams()); + return (script, service) -> new MapperScript<>(script) { + + final LongFieldScript.Factory scriptFactory = service.compile(script, LongFieldScript.CONTEXT); + + @Override + public void executeAndEmit(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { + LongFieldScript s = scriptFactory + .newFactory(fieldName, script.getParams(), lookup) + .newInstance(ctx); + s.runForDoc(doc); + long[] vs = s.values(); + for (int i = 0; i < s.count(); i++) { + emitter.accept(vs[i]); + } + } + }; } @Override @@ -1041,7 +1068,6 @@ public void setNextReader(LeafReaderContext context) { public List fetchValues(SourceLookup lookup) { List values = new ArrayList<>(); script.executeAndEmit(context.lookup(), ctx, lookup.docId(), values::add); - values.sort(Comparator.comparingLong(a -> (Long) a)); return values; } }; @@ -1204,66 +1230,4 @@ public PostParseExecutor getPostParseExecutor() { public FieldMapper.Builder getMergeBuilder() { return new Builder(simpleName(), type, ignoreMalformedByDefault, coerceByDefault).init(this); } - - private static class LongMapperScript extends MapperScript { - - final LongFieldScript.Factory scriptFactory; - final Map scriptParams; - final String fieldName; - - private LongMapperScript( - String fieldName, - Script script, - LongFieldScript.Factory scriptFactory, - Map scriptParams - ) { - super(script); - this.scriptFactory = scriptFactory; - this.scriptParams = scriptParams; - this.fieldName = fieldName; - } - - @Override - public void executeAndEmit(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { - LongFieldScript s = scriptFactory - .newFactory(fieldName, scriptParams, lookup) - .newInstance(ctx); - s.runForDoc(doc); - long[] vs = s.values(); - for (int i = 0; i < s.count(); i++) { - emitter.accept(vs[i]); - } - } - } - - private static class DoubleMapperScript extends MapperScript { - - final DoubleFieldScript.Factory scriptFactory; - final Map scriptParams; - final String fieldName; - - private DoubleMapperScript( - String fieldName, - Script script, - DoubleFieldScript.Factory scriptFactory, - Map scriptParams - ) { - super(script); - this.scriptFactory = scriptFactory; - this.scriptParams = scriptParams; - this.fieldName = fieldName; - } - - @Override - public void executeAndEmit(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { - DoubleFieldScript s = scriptFactory - .newFactory(fieldName, scriptParams, lookup) - .newInstance(ctx); - s.runForDoc(doc); - double[] vs = s.values(); - for (int i = 0; i < s.count(); i++) { - emitter.accept(vs[i]); - } - } - } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java index 3079d9cc82658..0b802eaba9486 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java @@ -569,5 +569,4 @@ void toXContent(XContentBuilder builder, Params params, ToXContent custom) throw protected void doXContent(XContentBuilder builder, Params params) throws IOException { } - } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java b/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java deleted file mode 100644 index afbd1b70b55b4..0000000000000 --- a/server/src/main/java/org/elasticsearch/index/mapper/ScriptParams.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.index.mapper; - -import org.elasticsearch.script.Script; -import org.elasticsearch.script.ScriptService; -import org.elasticsearch.script.ScriptType; - -import java.util.function.BiFunction; -import java.util.function.Function; - -/** - * Static methods for mapping parameters relating to mapper scripts - */ -public final class ScriptParams { - - private ScriptParams() { } - - /** - * Defines a script parameter - * @param compiler a function that produces a compiled script, given a {@link Script} and {@link ScriptService} - * @param initializer retrieves the equivalent parameter from an existing FieldMapper for use in merges - * @param the type of the compiled script - * @return a script parameter - */ - public static FieldMapper.Parameter> script( - BiFunction> compiler, - Function>initializer - ) { - return new FieldMapper.Parameter<>( - "script", - false, - () -> null, - (n, c, o) -> { - if (o == null) { - return null; - } - Script script = Script.parse(o); - if (script.getType() == ScriptType.STORED) { - throw new IllegalArgumentException("stored scripts are not supported on scripted field [" + n + "]"); - } - return compiler.apply(script, c.scriptService()); - }, - initializer - ).acceptsNull(); - } -} From 152faacbb9c257dd3db33c67003b9fa3d2a3da0b Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 16 Mar 2021 10:54:30 +0000 Subject: [PATCH 16/39] better error message --- .../java/org/elasticsearch/index/mapper/NumberFieldMapper.java | 2 +- .../org/elasticsearch/index/mapper/CalculatedFieldTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 5e6ababe4eeb7..8db1121d2735e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -1161,7 +1161,7 @@ protected String contentType() { protected void parseCreateField(ParseContext context) throws IOException { if (this.script != null) { - throw new IllegalArgumentException("Cannot index data directly into scripted field"); + throw new IllegalArgumentException("Cannot index data directly into a field with a [script] parameter"); } XContentParser parser = context.parser(); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index a41b4e6e5316a..9ae6f091caf1b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -132,7 +132,7 @@ public void testCannotIndexDirectlyIntoScriptMapper() throws IOException { assertEquals("failed to parse field [message_length] of type [long] in document with id '1'. Preview of field's value: '3'", e.getMessage()); Throwable original = e.getCause(); - assertEquals("Cannot index data directly into scripted field", original.getMessage()); + assertEquals("Cannot index data directly into a field with a [script] parameter", original.getMessage()); } public void testLoopDetection() throws IOException { From 4d3d4f10fcefba53d5d4907acf77ba4530311835 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 16 Mar 2021 11:41:08 +0000 Subject: [PATCH 17/39] Throw error if we try to refer to runtime fields in indextime scripts --- .../index/mapper/MappingLookup.java | 14 ++++++++++---- .../index/mapper/PostParseContext.java | 6 ++++-- .../index/mapper/PostParsePhase.java | 16 ++++++++++------ .../index/mapper/CalculatedFieldTests.java | 5 ++--- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java index e02371afa551c..67e6b8ab58ba1 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -54,6 +54,7 @@ private CacheKey() {} private final Map objectMappers; private final boolean hasNested; private final FieldTypeLookup fieldTypeLookup; + private final FieldTypeLookup indexTimeLookup; private final Map indexAnalyzersMap = new HashMap<>(); private final Map postParsePhases = new HashMap<>(); private final DocumentParser documentParser; @@ -154,6 +155,7 @@ public MappingLookup(Mapping mapping, } this.fieldTypeLookup = new FieldTypeLookup(mappers, aliasMappers, mapping.getRoot().runtimeFieldTypes()); + this.indexTimeLookup = postParsePhases.isEmpty() ? null : new FieldTypeLookup(mappers, aliasMappers, Collections.emptyList()); this.fieldMappers = Collections.unmodifiableMap(fieldMappers); this.objectMappers = Collections.unmodifiableMap(objects); } @@ -172,6 +174,14 @@ FieldTypeLookup fieldTypesLookup() { return fieldTypeLookup; } + PostParsePhase buildPostParsePhase(ParseContext pc) { + if (postParsePhases.isEmpty()) { + return null; + } + assert indexTimeLookup != null; + return new PostParsePhase(postParsePhases, indexTimeLookup::get, pc); + } + public NamedAnalyzer indexAnalyzer(String field, Function unmappedFieldAnalyzer) { if (this.indexAnalyzersMap.containsKey(field)) { return this.indexAnalyzersMap.get(field); @@ -179,10 +189,6 @@ public NamedAnalyzer indexAnalyzer(String field, Function return unmappedFieldAnalyzer.apply(field); } - public Map getPostParseExecutors() { - return postParsePhases; - } - /** * Returns an iterable over all the registered field mappers (including alias mappers) */ diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java index eab8069f321bc..d231cdade692b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java @@ -13,15 +13,17 @@ import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.search.lookup.SearchLookup; +import java.util.function.Function; + public class PostParseContext { public final SearchLookup searchLookup; public final LeafReaderContext leafReaderContext; public final ParseContext pc; - public PostParseContext(MappingLookup mappingLookup, ParseContext pc, LeafReaderContext ctx) { + public PostParseContext(Function fieldTypeLookup, ParseContext pc, LeafReaderContext ctx) { this.searchLookup = new SearchLookup( - mappingLookup::getFieldType, + fieldTypeLookup, (ft, s) -> ft.fielddataBuilder(pc.indexSettings().getIndex().getName(), s).build( new IndexFieldDataCache.None(), new NoneCircuitBreakerService()) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java index c566d5adbf674..97aa812945277 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java @@ -36,6 +36,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; /** @@ -52,22 +53,25 @@ public class PostParsePhase { * @param parseContext the ParseContext of the current document */ public static void executePostParsePhases(MappingLookup lookup, ParseContext parseContext) { - if (lookup.getPostParseExecutors().isEmpty()) { + PostParsePhase postParsePhase = lookup.buildPostParsePhase(parseContext); + if (postParsePhase == null) { return; } - PostParsePhase postParsePhase = new PostParsePhase(lookup, parseContext); for (OneTimeFieldExecutor executor : postParsePhase.fieldExecutors.values()) { executor.execute(); } } - private PostParsePhase(MappingLookup mappingLookup, ParseContext pc) { + PostParsePhase( + Map postParseExecutors, + Function fieldTypeLookup, + ParseContext pc) { LazyDocumentReader reader = new LazyDocumentReader( pc.rootDoc(), pc.sourceToParse().source(), - mappingLookup.getPostParseExecutors().keySet()); - this.context = new PostParseContext(mappingLookup, pc, reader.getContext()); - mappingLookup.getPostParseExecutors().forEach((k, c) -> fieldExecutors.put(k, new OneTimeFieldExecutor(c))); + postParseExecutors.keySet()); + this.context = new PostParseContext(fieldTypeLookup, pc, reader.getContext()); + postParseExecutors.forEach((k, c) -> fieldExecutors.put(k, new OneTimeFieldExecutor(c))); } // FieldExecutors can be called both by executePostParse() and from the lazy reader, diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index 9ae6f091caf1b..a942cc4561b54 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -157,7 +157,6 @@ public void testStoredScriptsNotPermitted() { assertThat(e.getMessage(), equalTo("Failed to parse mapping: stored scripts are not supported on scripted field [field]")); } - @AwaitsFix(bugUrl = "TODO") public void testCannotReferToRuntimeFields() throws IOException { DocumentMapper mapper = createDocumentMapper(topMapping(b -> { b.startObject("runtime"); @@ -168,8 +167,8 @@ public void testCannotReferToRuntimeFields() throws IOException { b.endObject(); })); - Exception e = expectThrows(IllegalArgumentException.class, () -> mapper.parse(source(b -> {}))); - assertEquals("Cannot reference runtime field [runtime-field] in an index-time script", e.getMessage()); + Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> {}))); + assertEquals("No field found for [runtime-field] in mapping", e.getCause().getMessage()); } private static Consumer getLongScript(String name) { From 9791f94847dd6e56bd4faec77f110cac38cd7bf3 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 16 Mar 2021 12:15:39 +0000 Subject: [PATCH 18/39] Add fetch tests for long/double index-time script fields with no docvalues --- .../runtime_fields/23_long_calculated_at_index.yml | 14 +++++++++++++- .../33_double_calculated_at_index.yml | 13 ++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml index b7f4b85814488..3a0b36e5c3d5d 100644 --- a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml +++ b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml @@ -26,7 +26,17 @@ setup: } params: multiplier: 10 - # Test fetching many values + voltage_times_ten_no_dv: + type: long + doc_values: false + script: + source: | + for (double v : doc['voltage']) { + emit((long)(v * params.multiplier)); + } + params: + multiplier: 10 + # test multiple values temperature_digits: type: long script: @@ -80,11 +90,13 @@ setup: sort: timestamp fields: - voltage_times_ten + - voltage_times_ten_no_dv - temperature_digits - match: {hits.total.value: 6} - match: {hits.hits.0.fields.voltage_times_ten: [40] } - match: {hits.hits.0.fields.temperature_digits: [0, 2, 2] } - match: {hits.hits.0.fields.voltage_times_ten: [40] } + - match: {hits.hits.0.fields.voltage_times_ten_no_dv: [40] } - match: {hits.hits.1.fields.voltage_times_ten: [42] } - match: {hits.hits.2.fields.voltage_times_ten: [56] } - match: {hits.hits.3.fields.voltage_times_ten: [51] } diff --git a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml index be9f3bfeac016..78c994dbe57da 100644 --- a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml +++ b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml @@ -26,6 +26,16 @@ setup: } params: max: 5.8 + voltage_percent_no_dv: + type: double + doc_values: false + script: + source: | + for (double v : doc['voltage']) { + emit(v / params.max); + } + params: + max: 5.8 # Test fetching many values voltage_sqrts: type: double @@ -78,12 +88,13 @@ setup: index: sensor body: sort: timestamp - fields: [voltage_percent, voltage_sqrts] + fields: [voltage_percent, voltage_percent_no_dv, voltage_sqrts] - match: {hits.total.value: 6} - match: {hits.hits.0.fields.voltage_percent: [0.6896551724137931] } # 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] } + - match: {hits.hits.1.fields.voltage_percent_no_dv: [0.7241379310344828] } - match: {hits.hits.2.fields.voltage_percent: [0.9655172413793103] } - match: {hits.hits.3.fields.voltage_percent: [0.8793103448275862] } - match: {hits.hits.4.fields.voltage_percent: [1.0] } From 918dc6a6d523b5d20cc0da7e029f714c74c7bb51 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 16 Mar 2021 13:29:15 +0000 Subject: [PATCH 19/39] javadocs --- .../elasticsearch/index/mapper/PostParseContext.java | 10 +++++++++- .../elasticsearch/index/mapper/PostParseExecutor.java | 3 +++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java index d231cdade692b..1cd02be151133 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java @@ -15,13 +15,21 @@ import java.util.function.Function; +/** + * Holds state useful for post-parse document processing + */ public class PostParseContext { + /** A search lookup over the parsed document */ public final SearchLookup searchLookup; + + /** A LeafReaderContext for a lucene reader based on the parsed document */ public final LeafReaderContext leafReaderContext; + + /** The ParseContext used during document parsing */ public final ParseContext pc; - public PostParseContext(Function fieldTypeLookup, ParseContext pc, LeafReaderContext ctx) { + PostParseContext(Function fieldTypeLookup, ParseContext pc, LeafReaderContext ctx) { this.searchLookup = new SearchLookup( fieldTypeLookup, (ft, s) -> ft.fielddataBuilder(pc.indexSettings().getIndex().getName(), s).build( diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java index 0cd7477e24ebf..621fd125732c8 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java @@ -8,6 +8,9 @@ package org.elasticsearch.index.mapper; +/** + * Runs after a document has been parsed + */ public interface PostParseExecutor { void execute(PostParseContext context); From 1917ddb25126d5b9bc0cadd95313fb335b94b841 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 18 Mar 2021 13:44:45 +0000 Subject: [PATCH 20/39] Add test to MapperTestCase that checks fielddata returned via LazyDocumentReader --- .../index/mapper/PostParsePhase.java | 24 +++++--- .../index/mapper/DoubleFieldMapperTests.java | 5 ++ .../index/mapper/LongFieldMapperTests.java | 5 ++ .../index/mapper/NumberFieldMapperTests.java | 36 +++++++----- .../index/mapper/RangeFieldMapperTests.java | 5 ++ .../index/mapper/MapperTestCase.java | 56 +++++++++++++++++++ 6 files changed, 110 insertions(+), 21 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java index 97aa812945277..a94c13c858376 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java @@ -57,9 +57,7 @@ public static void executePostParsePhases(MappingLookup lookup, ParseContext par if (postParsePhase == null) { return; } - for (OneTimeFieldExecutor executor : postParsePhase.fieldExecutors.values()) { - executor.execute(); - } + postParsePhase.execute(); } PostParsePhase( @@ -74,6 +72,12 @@ public static void executePostParsePhases(MappingLookup lookup, ParseContext par postParseExecutors.forEach((k, c) -> fieldExecutors.put(k, new OneTimeFieldExecutor(c))); } + void execute() { + for (OneTimeFieldExecutor executor : fieldExecutors.values()) { + executor.execute(); + } + } + // FieldExecutors can be called both by executePostParse() and from the lazy reader, // so to ensure that we don't run field scripts multiple times we guard them with // this one-time executor class @@ -150,7 +154,7 @@ public SortedDocValues getSortedDocValues(String field) throws IOException { checkField(field); List values = document.getFields().stream() .filter(f -> Objects.equals(f.name(), field)) - .filter(f -> f.fieldType().docValuesType() == DocValuesType.BINARY) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.SORTED) .map(IndexableField::binaryValue) .sorted() .collect(Collectors.toList()); @@ -173,7 +177,7 @@ public SortedNumericDocValues getSortedNumericDocValues(String field) throws IOE public SortedSetDocValues getSortedSetDocValues(String field) throws IOException { List values = document.getFields().stream() .filter(f -> Objects.equals(f.name(), field)) - .filter(f -> f.fieldType().docValuesType() == DocValuesType.BINARY) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.SORTED_SET) .map(IndexableField::binaryValue) .sorted() .collect(Collectors.toList()); @@ -182,7 +186,7 @@ public SortedSetDocValues getSortedSetDocValues(String field) throws IOException @Override public FieldInfos getFieldInfos() { - throw new UnsupportedOperationException(); + return new FieldInfos(new FieldInfo[0]); } @Override @@ -362,7 +366,7 @@ public BytesRef binaryValue() { @Override public boolean advanceExact(int target) { - return false; + return true; } @Override @@ -446,6 +450,9 @@ private static SortedSetDocValues sortedSetDocValues(List values) { @Override public long nextOrd() { i++; + if (i >= values.size()) { + return NO_MORE_ORDS; + } return i; } @@ -461,6 +468,7 @@ public long getValueCount() { @Override public boolean advanceExact(int target) { + i = -1; return true; } @@ -471,11 +479,13 @@ public int docID() { @Override public int nextDoc() { + i = -1; return 0; } @Override public int advance(int target) { + i = -1; return 0; } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java index 8425d18adfa4a..0af374f00f053 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java @@ -36,4 +36,9 @@ protected List outOfRangeSpecs() { protected void minimalMapping(XContentBuilder b) throws IOException { b.field("type", "double"); } + + @Override + protected boolean allowsIndexTimeScript() { + return true; + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java index 2c45d38dd8124..c6a77b93ca086 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java @@ -43,6 +43,11 @@ protected void minimalMapping(XContentBuilder b) throws IOException { b.field("type", "long"); } + @Override + protected boolean allowsIndexTimeScript() { + return true; + } + public void testLongIndexingOutOfRange() throws Exception { DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "long").field("ignore_malformed", true))); ParsedDocument doc = mapper.parse( diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java index bbeca2c44297b..97dcc2b552ad6 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -32,6 +32,13 @@ public abstract class NumberFieldMapperTests extends MapperTestCase { */ protected abstract Number missingValue(); + /** + * @return does this mapper allow index time scripts + */ + protected boolean allowsIndexTimeScript() { + return false; + } + @Override protected void registerParameters(ParameterChecker checker) throws IOException { checker.registerConflictCheck("doc_values", b -> b.field("doc_values", false)); @@ -205,7 +212,7 @@ protected void testNullValue() throws IOException { assertEquals(DocValuesType.SORTED_NUMERIC, dvField.fieldType().docValuesType()); assertFalse(dvField.fieldType().stored()); } - + public void testOutOfRangeValues() throws IOException { for(OutOfRangeSpec item : outOfRangeSpecs()) { @@ -222,19 +229,20 @@ public void testOutOfRangeValues() throws IOException { } public void testScriptableTypes() { - Set scriptableTypes = Set.of("long", "double"); - for (String type : types()) { - if (scriptableTypes.contains(type)) { - // won't actually compile because we don't have painless in unit tests, but we can - // check that it gets as far as trying to compile it - Exception e = expectThrows(MapperParsingException.class, - () -> createDocumentMapper(fieldMapping(b -> b.field("type", type).field("script", "foo")))); - assertEquals("Failed to parse mapping: script_lang not supported [painless]", e.getMessage()); - } else { - Exception e = expectThrows(MapperParsingException.class, "Missing exception on type " + type, - () -> createDocumentMapper(fieldMapping(b -> b.field("type", type).field("script", "foo")))); - assertEquals("Failed to parse mapping: Unknown parameter [script] for mapper [field]", e.getMessage()); - } + if (allowsIndexTimeScript()) { + // won't actually compile because we don't have painless in unit tests, but we can + // check that it gets as far as trying to compile it + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("script", "foo"); + }))); + assertEquals("Failed to parse mapping: script_lang not supported [painless]", e.getMessage()); + } else { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + minimalMapping(b); + b.field("script", "foo"); + }))); + assertEquals("Failed to parse mapping: Unknown parameter [script] for mapper [field]", e.getMessage()); } } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java index 0fda397e3e621..380570c7441e2 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java @@ -68,6 +68,11 @@ protected Object getSampleValueForQuery() { return 6; } + @Override + public void testIndexTimeFieldData() throws IOException { + assumeFalse("Accessing range fields in scripts is not supported", false); + } + public void testExistsQueryDocValuesDisabled() throws IOException { MapperService mapperService = createMapperService(fieldMapping(b -> { minimalMapping(b); diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index ce9b362494b59..0bbae3daa5301 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.IndexFieldDataCache; +import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.search.DocValueFormat; @@ -36,6 +37,7 @@ import org.elasticsearch.search.lookup.SourceLookup; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -48,6 +50,7 @@ import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -481,4 +484,57 @@ protected void assertFetch(MapperService mapperService, String field, Object val assertEquals(fromDocValues, fromNative); }); } + + public void testIndexTimeFieldData() throws IOException { + MapperService mapperService = createMapperService(fieldMapping(this::minimalMapping)); + assertParseMinimalWarnings(); + MappedFieldType fieldType = mapperService.fieldType("field"); + if (fieldType.isAggregatable() == false) { + return; // No field data available, so we ignore + } + SourceToParse source = source(this::writeField); + ParsedDocument doc = mapperService.documentMapper().parse(source); + + withLuceneIndex(mapperService, iw -> iw.addDocument(doc.rootDoc()), ir -> { + + LeafReaderContext ctx = ir.leaves().get(0); + + ScriptDocValues fieldData = fieldType + .fielddataBuilder("test", () -> { throw new UnsupportedOperationException(); }) + .build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService()) + .load(ctx) + .getScriptValues(); + + fieldData.setNextDocId(0); + + PostParseExecutor indexTimeExecutor = context -> { + try { + ScriptDocValues indexData = fieldType + .fielddataBuilder("test", () -> { + throw new UnsupportedOperationException(); + }) + .build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService()) + .load(context.leafReaderContext) + .getScriptValues(); + indexData.setNextDocId(0); + + // compare index and search time fielddata + assertThat(fieldData, equalTo(indexData)); + + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }; + + ParseContext pc = mock(ParseContext.class); + when(pc.rootDoc()).thenReturn(doc.rootDoc()); + when(pc.sourceToParse()).thenReturn(source); + + PostParsePhase postParsePhase = new PostParsePhase( + Map.of("test", indexTimeExecutor), + mapperService::fieldType, + pc); + postParsePhase.execute(); + }); + } } From 7981259cb1467d34994558c41a9bfc63fc38ef1b Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 18 Mar 2021 14:43:19 +0000 Subject: [PATCH 21/39] Allow test case to say if field does not support scripts --- .../index/mapper/RangeFieldMapperTests.java | 4 ++-- .../org/elasticsearch/index/mapper/MapperTestCase.java | 10 +++++++++- .../analytics/mapper/HistogramFieldMapperTests.java | 5 +++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java index 380570c7441e2..bec6c1caff9e0 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldMapperTests.java @@ -69,8 +69,8 @@ protected Object getSampleValueForQuery() { } @Override - public void testIndexTimeFieldData() throws IOException { - assumeFalse("Accessing range fields in scripts is not supported", false); + protected boolean supportsScripts() { + return false; } public void testExistsQueryDocValuesDisabled() throws IOException { diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index 0bbae3daa5301..1690991bedf2a 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -485,7 +485,15 @@ protected void assertFetch(MapperService mapperService, String field, Object val }); } - public void testIndexTimeFieldData() throws IOException { + /** + * @return whether or not this field type supports access to its values from scripts + */ + protected boolean supportsScripts() { + return true; + } + + public final void testIndexTimeFieldData() throws IOException { + assumeTrue("Field type does not support scripting", supportsScripts()); MapperService mapperService = createMapperService(fieldMapping(this::minimalMapping)); assertParseMinimalWarnings(); MappedFieldType fieldType = mapperService.fieldType("field"); diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java index 2ef401206ef04..e9c362dd335e6 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java @@ -48,6 +48,11 @@ protected void registerParameters(ParameterChecker checker) throws IOException { m -> assertTrue(((HistogramFieldMapper)m).ignoreMalformed())); } + @Override + protected boolean supportsScripts() { + return false; + } + public void testParseValue() throws Exception { DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping)); ParsedDocument doc = mapper.parse( From b8ade1a934221ef44ef98aaee353993f7f0ad8e0 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 18 Mar 2021 15:04:40 +0000 Subject: [PATCH 22/39] unsigned long doesn't support scripts --- .../xpack/unsignedlong/UnsignedLongFieldMapperTests.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java index 1afb898a59eb4..9bd38e43df20c 100644 --- a/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java +++ b/x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapperTests.java @@ -43,6 +43,11 @@ protected Object getSampleValueForDocument() { return 123; } + @Override + protected boolean supportsScripts() { + return false; + } + @Override protected void registerParameters(ParameterChecker checker) throws IOException { checker.registerConflictCheck("doc_values", b -> b.field("doc_values", false)); From 45c0bab4d984889ae51649c2bb842a23713b7fb4 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 18 Mar 2021 15:34:31 +0000 Subject: [PATCH 23/39] or geoshapewithdocvalues --- .../index/mapper/GeoShapeWithDocValuesFieldMapperTests.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java index 65b4661410d19..5874c0e54035e 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java @@ -54,6 +54,11 @@ protected Object getSampleValueForDocument() { return "POINT (14.0 15.0)"; } + @Override + protected boolean supportsScripts() { + return false; + } + @Override protected void registerParameters(ParameterChecker checker) throws IOException { checker.registerConflictCheck("doc_values", b -> b.field("doc_values", false)); From ea2cef55d815430e937d1ebf76afc50c2f22fd6d Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 22 Mar 2021 09:58:44 +0000 Subject: [PATCH 24/39] Error handling --- .../index/mapper/NumberFieldMapper.java | 37 +++++++++++--- .../index/mapper/PostParseExecutor.java | 6 ++- .../index/mapper/PostParsePhase.java | 14 +++-- .../index/mapper/CalculatedFieldTests.java | 51 +++++++++++++++++++ .../index/mapper/MapperTestCase.java | 12 +++-- 5 files changed, 101 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 8db1121d2735e..5c940e6779c68 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -81,6 +81,12 @@ public static class Builder extends FieldMapper.Builder { private final Parameter nullValue; private final Parameter> script; + private final Parameter onScriptError = Parameter.restrictedStringParam( + "on_script_error", + false, + m -> toType(m).onScriptError, + "reject", "ignore" + ); private final Parameter> meta = Parameter.metaParam(); @@ -123,7 +129,7 @@ public Builder docValues(boolean hasDocValues) { @Override protected List> getParameters() { - return List.of(indexed, hasDocValues, stored, ignoreMalformed, coerce, nullValue, script, meta); + return List.of(indexed, hasDocValues, stored, ignoreMalformed, coerce, nullValue, script, onScriptError, meta); } @Override @@ -983,7 +989,7 @@ public NumberFieldType(String name, NumberType type, boolean isSearchable, boole this.type = Objects.requireNonNull(type); this.coerce = coerce; this.nullValue = nullValue; - this.script = script == null ? null : script; + this.script = script; } NumberFieldType(String name, Builder builder) { @@ -1116,6 +1122,7 @@ public CollapseType collapseType() { private final Explicit coerce; private final Number nullValue; private final MapperScript script; + private final String onScriptError; private final boolean ignoreMalformedByDefault; private final boolean coerceByDefault; @@ -1137,6 +1144,7 @@ private NumberFieldMapper( this.ignoreMalformedByDefault = builder.ignoreMalformed.getDefaultValue().value(); this.coerceByDefault = builder.coerce.getDefaultValue().value(); this.script = builder.script.get(); + this.onScriptError = builder.onScriptError.get(); } boolean coerce() { @@ -1218,12 +1226,25 @@ public PostParseExecutor getPostParseExecutor() { if (this.script == null) { return null; } - return c -> this.script.executeAndEmit( - c.searchLookup, - c.leafReaderContext, - 0, - v -> this.indexValue(c.pc, v) - ); + return new PostParseExecutor() { + @Override + public void execute(PostParseContext context) { + script.executeAndEmit( + context.searchLookup, + context.leafReaderContext, + 0, + v -> indexValue(context.pc, v)); + } + + @Override + public void onError(PostParseContext context, Exception e) throws IOException { + if ("ignore".equals(onScriptError)) { + context.pc.addIgnoredField(name()); + } else { + throw new IOException("Error executing script on field [" + name() + "]", e); + } + } + }; } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java index 621fd125732c8..5dd804d06ec53 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java @@ -8,11 +8,15 @@ package org.elasticsearch.index.mapper; +import java.io.IOException; + /** * Runs after a document has been parsed */ public interface PostParseExecutor { - void execute(PostParseContext context); + void execute(PostParseContext context) throws IOException; + + void onError(PostParseContext context, Exception e) throws IOException; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java index a94c13c858376..156d7ff8f49d3 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java @@ -52,7 +52,7 @@ public class PostParsePhase { * @param lookup the MappingLookup to collect executors from * @param parseContext the ParseContext of the current document */ - public static void executePostParsePhases(MappingLookup lookup, ParseContext parseContext) { + public static void executePostParsePhases(MappingLookup lookup, ParseContext parseContext) throws IOException { PostParsePhase postParsePhase = lookup.buildPostParsePhase(parseContext); if (postParsePhase == null) { return; @@ -72,7 +72,7 @@ public static void executePostParsePhases(MappingLookup lookup, ParseContext par postParseExecutors.forEach((k, c) -> fieldExecutors.put(k, new OneTimeFieldExecutor(c))); } - void execute() { + void execute() throws IOException { for (OneTimeFieldExecutor executor : fieldExecutors.values()) { executor.execute(); } @@ -90,9 +90,13 @@ private class OneTimeFieldExecutor { this.executor = executor; } - void execute() { + void execute() throws IOException { if (executed == false) { - executor.execute(context); + try { + executor.execute(context); + } catch (Exception e) { + executor.onError(context, e); + } executed = true; } } @@ -111,7 +115,7 @@ private LazyDocumentReader(ParseContext.Document document, BytesReference source this.calculatedFields = calculatedFields; } - private void checkField(String field) { + private void checkField(String field) throws IOException { if (calculatedFields.contains(field)) { // this means that a mapper script is referring to another calculated field; // in which case we need to execute that field first. We also check for loops here diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index a942cc4561b54..7a2c4e916ed7c 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -28,6 +28,7 @@ import java.util.Set; import java.util.function.Consumer; +import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -171,12 +172,62 @@ public void testCannotReferToRuntimeFields() throws IOException { assertEquals("No field found for [runtime-field] in mapping", e.getCause().getMessage()); } + public void testIgnoreScriptErrors() throws IOException { + DocumentMapper mapper = createDocumentMapper(mapping(b -> { + b.startObject("message").field("type", "keyword").endObject(); + b.startObject("message_length"); + { + b.field("type", "long"); + b.field("script", "message_length"); + } + b.endObject(); + b.startObject("message_error"); + { + b.field("type", "long"); + b.field("script", "throws"); + b.field("on_script_error", "ignore"); + } + b.endObject(); + })); + + ParsedDocument doc = mapper.parse(source(b -> b.field("message", "this is some text"))); + assertThat(doc.rootDoc().getFields("message_length"), arrayWithSize(2)); + assertThat(doc.rootDoc().getFields("message_error"), arrayWithSize(0)); + assertThat(doc.rootDoc().getField("_ignored").stringValue(), equalTo("message_error")); + } + + public void testRejectScriptErrors() throws IOException { + DocumentMapper mapper = createDocumentMapper(mapping(b -> { + b.startObject("message").field("type", "keyword").endObject(); + b.startObject("message_length"); + { + b.field("type", "long"); + b.field("script", "message_length"); + } + b.endObject(); + b.startObject("message_error"); + { + b.field("type", "long"); + b.field("script", "throws"); + } + b.endObject(); + })); + + Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.field("message", "foo")))); + assertThat(e.getMessage(), equalTo("failed to parse")); + assertThat(e.getCause().getMessage(), equalTo("Error executing script on field [message_error]")); + assertThat(e.getCause().getCause().getMessage(), equalTo("Oops!")); + } + private static Consumer getLongScript(String name) { if ("refer-to-runtime".equals(name)) { return s -> { s.emitValue((long) s.getDoc().get("runtime-field").get(0)); }; } + if ("throws".equals(name)) { + return s -> { throw new RuntimeException("Oops!"); }; + } if (name.endsWith("_length")) { String field = name.substring(0, name.lastIndexOf("_length")); return s -> { diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index 1690991bedf2a..4ba779a243219 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -37,7 +37,6 @@ import org.elasticsearch.search.lookup.SourceLookup; import java.io.IOException; -import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -515,8 +514,9 @@ public final void testIndexTimeFieldData() throws IOException { fieldData.setNextDocId(0); - PostParseExecutor indexTimeExecutor = context -> { - try { + PostParseExecutor indexTimeExecutor = new PostParseExecutor() { + @Override + public void execute(PostParseContext context) throws IOException { ScriptDocValues indexData = fieldType .fielddataBuilder("test", () -> { throw new UnsupportedOperationException(); @@ -528,9 +528,11 @@ public final void testIndexTimeFieldData() throws IOException { // compare index and search time fielddata assertThat(fieldData, equalTo(indexData)); + } - } catch (IOException e) { - throw new UncheckedIOException(e); + @Override + public void onError(PostParseContext context, Exception e) throws IOException { + throw new IOException(e); } }; From 4a08638d03f35a4c107dedecb90ef18e335f5b39 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 22 Mar 2021 10:11:53 +0000 Subject: [PATCH 25/39] Don't load from docvalues at fetch time --- .../elasticsearch/index/mapper/NumberFieldMapper.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 5c940e6779c68..2f31d2a6f2f86 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -1058,10 +1058,6 @@ public ValueFetcher valueFetcher(SearchExecutionContext context, String format) throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() + "] doesn't support formats."); } if (this.script != null) { - // values don't live in source - either pull from dv, or re-calculate - if (hasDocValues()) { - return new DocValueFetcher(DocValueFormat.RAW, context.getForField(this)); - } return new ValueFetcher() { LeafReaderContext ctx; @@ -1073,7 +1069,12 @@ public void setNextReader(LeafReaderContext context) { @Override public List fetchValues(SourceLookup lookup) { List values = new ArrayList<>(); - script.executeAndEmit(context.lookup(), ctx, lookup.docId(), values::add); + try { + script.executeAndEmit(context.lookup(), ctx, lookup.docId(), values::add); + } catch (Exception e) { + // ignore errors - if they exist here then they existed at index time + // and were ignored, so we ignore here too + } return values; } }; From cff82811b8877b87428703cb2f73ae31986872f7 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 22 Mar 2021 11:34:13 +0000 Subject: [PATCH 26/39] tests --- .../index/mapper/CalculatedFieldTests.java | 13 ++++++++++--- .../index/mapper/DoubleFieldMapperTests.java | 2 +- .../index/mapper/LongFieldMapperTests.java | 2 +- .../index/mapper/NumberFieldMapperTests.java | 9 +-------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index 7a2c4e916ed7c..bc42a419add8e 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -144,10 +144,16 @@ public void testLoopDetection() throws IOException { Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> {}))); assertEquals("failed to parse", e.getMessage()); - assertThat(e.getCause().getMessage(), containsString("Loop in field resolution detected")); + assertEquals("Error executing script on field [field1]", e.getCause().getMessage()); + + Throwable root = e.getCause(); + while (root.getCause() != null) { + root = root.getCause(); + } + assertThat(root.getMessage(), containsString("Loop in field resolution detected")); // Can be either field1->field2->field1 or field2->field1->field2 because // post-phase executor order is not deterministic - assertThat(e.getCause().getMessage(), containsString("field1->field2")); + assertThat(root.getMessage(), containsString("field1->field2")); } public void testStoredScriptsNotPermitted() { @@ -169,7 +175,8 @@ public void testCannotReferToRuntimeFields() throws IOException { })); Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> {}))); - assertEquals("No field found for [runtime-field] in mapping", e.getCause().getMessage()); + assertEquals("Error executing script on field [index-field]", e.getCause().getMessage()); + assertEquals("No field found for [runtime-field] in mapping", e.getCause().getCause().getMessage()); } public void testIgnoreScriptErrors() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java index 0af374f00f053..73dc89535179e 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java @@ -38,7 +38,7 @@ protected void minimalMapping(XContentBuilder b) throws IOException { } @Override - protected boolean allowsIndexTimeScript() { + protected boolean supportsScripts() { return true; } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java index c6a77b93ca086..9879353c6d0ba 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java @@ -44,7 +44,7 @@ protected void minimalMapping(XContentBuilder b) throws IOException { } @Override - protected boolean allowsIndexTimeScript() { + protected boolean supportsScripts() { return true; } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java index 97dcc2b552ad6..9601353fa4f37 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -32,13 +32,6 @@ public abstract class NumberFieldMapperTests extends MapperTestCase { */ protected abstract Number missingValue(); - /** - * @return does this mapper allow index time scripts - */ - protected boolean allowsIndexTimeScript() { - return false; - } - @Override protected void registerParameters(ParameterChecker checker) throws IOException { checker.registerConflictCheck("doc_values", b -> b.field("doc_values", false)); @@ -229,7 +222,7 @@ public void testOutOfRangeValues() throws IOException { } public void testScriptableTypes() { - if (allowsIndexTimeScript()) { + if (supportsScripts()) { // won't actually compile because we don't have painless in unit tests, but we can // check that it gets as far as trying to compile it Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { From 1276a9aa2b857a3b2f9b606043197319f7fbdff8 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 22 Mar 2021 12:00:14 +0000 Subject: [PATCH 27/39] duh --- .../test/runtime_fields/23_long_calculated_at_index.yml | 2 +- .../index/mapper/DoubleFieldMapperTests.java | 2 +- .../elasticsearch/index/mapper/LongFieldMapperTests.java | 2 +- .../index/mapper/NumberFieldMapperTests.java | 9 ++++++++- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml index 3a0b36e5c3d5d..a6eff621344c1 100644 --- a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml +++ b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/23_long_calculated_at_index.yml @@ -94,7 +94,7 @@ setup: - temperature_digits - match: {hits.total.value: 6} - match: {hits.hits.0.fields.voltage_times_ten: [40] } - - match: {hits.hits.0.fields.temperature_digits: [0, 2, 2] } + - match: {hits.hits.0.fields.temperature_digits: [2, 0, 2] } - match: {hits.hits.0.fields.voltage_times_ten: [40] } - match: {hits.hits.0.fields.voltage_times_ten_no_dv: [40] } - match: {hits.hits.1.fields.voltage_times_ten: [42] } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java index 73dc89535179e..0af374f00f053 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DoubleFieldMapperTests.java @@ -38,7 +38,7 @@ protected void minimalMapping(XContentBuilder b) throws IOException { } @Override - protected boolean supportsScripts() { + protected boolean allowsIndexTimeScript() { return true; } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java index 9879353c6d0ba..c6a77b93ca086 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java @@ -44,7 +44,7 @@ protected void minimalMapping(XContentBuilder b) throws IOException { } @Override - protected boolean supportsScripts() { + protected boolean allowsIndexTimeScript() { return true; } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java index 9601353fa4f37..97dcc2b552ad6 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -32,6 +32,13 @@ public abstract class NumberFieldMapperTests extends MapperTestCase { */ protected abstract Number missingValue(); + /** + * @return does this mapper allow index time scripts + */ + protected boolean allowsIndexTimeScript() { + return false; + } + @Override protected void registerParameters(ParameterChecker checker) throws IOException { checker.registerConflictCheck("doc_values", b -> b.field("doc_values", false)); @@ -222,7 +229,7 @@ public void testOutOfRangeValues() throws IOException { } public void testScriptableTypes() { - if (supportsScripts()) { + if (allowsIndexTimeScript()) { // won't actually compile because we don't have painless in unit tests, but we can // check that it gets as far as trying to compile it Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { From 09f59665500a3c90d101adfecc40ec79be838985 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 22 Mar 2021 12:23:10 +0000 Subject: [PATCH 28/39] we don't sort emitted values --- .../test/runtime_fields/33_double_calculated_at_index.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml index 78c994dbe57da..15a8022f3c1b1 100644 --- a/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml +++ b/modules/runtime-fields-common/src/yamlRestTest/resources/rest-api-spec/test/runtime_fields/33_double_calculated_at_index.yml @@ -91,8 +91,8 @@ setup: fields: [voltage_percent, voltage_percent_no_dv, voltage_sqrts] - match: {hits.total.value: 6} - match: {hits.hits.0.fields.voltage_percent: [0.6896551724137931] } - # 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] } + # Scripts that scripts that emit multiple values are supported + - match: {hits.hits.0.fields.voltage_sqrts: [4.0, 2.0, 1.4142135623730951] } - match: {hits.hits.1.fields.voltage_percent: [0.7241379310344828] } - match: {hits.hits.1.fields.voltage_percent_no_dv: [0.7241379310344828] } - match: {hits.hits.2.fields.voltage_percent: [0.9655172413793103] } From f68b5d86f315d64866321878f4f57c86a4b8b494 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 23 Mar 2021 12:03:13 +0000 Subject: [PATCH 29/39] Add parameter requirements and preclusions --- .../index/mapper/FieldMapper.java | 24 +++++++++++ .../index/mapper/NumberFieldMapper.java | 8 +++- .../index/mapper/CalculatedFieldTests.java | 9 ++++ .../index/mapper/LongFieldMapperTests.java | 31 ++++++++++++++ .../index/mapper/NumberFieldMapperTests.java | 42 ++++++++++++++++--- 5 files changed, 107 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index 489d89caab188..74a18e407a21a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -520,6 +520,8 @@ public static final class Parameter implements Supplier { private MergeValidator mergeValidator; private T value; private boolean isSet; + private List> requires = new ArrayList<>(); + private List> precludes = new ArrayList<>(); /** * Creates a new Parameter @@ -650,10 +652,32 @@ public Parameter setMergeValidator(MergeValidator mergeValidator) { return this; } + public Parameter requiresParameters(Parameter... ps) { + this.requires.addAll(Arrays.asList(ps)); + return this; + } + + public Parameter precludesParameters(Parameter... ps) { + this.precludes.addAll(Arrays.asList(ps)); + return this; + } + private void validate() { if (validator != null) { validator.accept(getValue()); } + if (this.isConfigured()) { + for (Parameter p : requires) { + if (p.isConfigured() == false) { + throw new IllegalArgumentException("Field [" + name + "] requires field [" + p.name + "] to be configured"); + } + } + for (Parameter p : precludes) { + if (p.isConfigured()) { + throw new IllegalArgumentException("Field [" + p.name + "] cannot be set in conjunction with field [" + name + "]"); + } + } + } } private void init(FieldMapper toInit) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 10373f50754ec..9ecf9cc6e2f70 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -83,7 +83,7 @@ public static class Builder extends FieldMapper.Builder { private final Parameter> script; private final Parameter onScriptError = Parameter.restrictedStringParam( "on_script_error", - false, + true, m -> toType(m).onScriptError, "reject", "ignore" ); @@ -115,6 +115,8 @@ public Builder(String name, NumberType type, boolean ignoreMalformedByDefault, b type.compiler(name), m -> toType(m).script ); + this.onScriptError.requiresParameters(this.script); + this.script.precludesParameters(ignoreMalformed, coerce, nullValue); } Builder nullValue(Number number) { @@ -1156,6 +1158,10 @@ boolean ignoreMalformed() { return ignoreMalformed.value(); } + String onScriptError() { + return onScriptError; + } + @Override public NumberFieldType fieldType() { return (NumberFieldType) super.fieldType(); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index 1fc1d59c58540..249cfca0a8b60 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -169,6 +169,15 @@ public void testCannotReferToRuntimeFields() throws IOException { assertEquals("No field found for [runtime-field] in mapping", e.getCause().getCause().getMessage()); } + public void testScriptErrorParameterRequiresScript() { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + b.field("type", "long"); + b.field("on_script_error", "ignore"); + }))); + assertThat(e.getMessage(), + equalTo("Failed to parse mapping: Field [on_script_error] requires field [script] to be configured")); + } + public void testIgnoreScriptErrors() throws IOException { DocumentMapper mapper = createDocumentMapper(mapping(b -> { b.startObject("message").field("type", "keyword").endObject(); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java index c6a77b93ca086..8fa88eed2c0e5 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LongFieldMapperTests.java @@ -18,6 +18,7 @@ import java.util.List; import static org.hamcrest.Matchers.arrayWithSize; +import static org.hamcrest.Matchers.equalTo; public class LongFieldMapperTests extends WholeNumberFieldMapperTests { @@ -48,6 +49,36 @@ protected boolean allowsIndexTimeScript() { return true; } + public void testScriptAndPrecludedParameters() { + { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + b.field("type", "long"); + b.field("script", "test"); + b.field("coerce", "true"); + }))); + assertThat(e.getMessage(), + equalTo("Failed to parse mapping: Field [coerce] cannot be set in conjunction with field [script]")); + } + { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + b.field("type", "long"); + b.field("script", "test"); + b.field("null_value", 7); + }))); + assertThat(e.getMessage(), + equalTo("Failed to parse mapping: Field [null_value] cannot be set in conjunction with field [script]")); + } + { + Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + b.field("type", "long"); + b.field("script", "test"); + b.field("ignore_malformed", "true"); + }))); + assertThat(e.getMessage(), + equalTo("Failed to parse mapping: Field [ignore_malformed] cannot be set in conjunction with field [script]")); + } + } + public void testLongIndexingOutOfRange() throws Exception { DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "long").field("ignore_malformed", true))); ParsedDocument doc = mapper.parse( diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java index 97dcc2b552ad6..8a105387768e7 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java @@ -14,11 +14,16 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.index.mapper.NumberFieldTypeTests.OutOfRangeSpec; import org.elasticsearch.index.termvectors.TermVectorsService; +import org.elasticsearch.runtimefields.mapper.DoubleFieldScript; +import org.elasticsearch.runtimefields.mapper.LongFieldScript; +import org.elasticsearch.script.Script; +import org.elasticsearch.script.ScriptContext; import java.io.IOException; import java.util.List; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; public abstract class NumberFieldMapperTests extends MapperTestCase { @@ -49,6 +54,22 @@ protected void registerParameters(ParameterChecker checker) throws IOException { m -> assertFalse(((NumberFieldMapper) m).coerce())); checker.registerUpdateCheck(b -> b.field("ignore_malformed", true), m -> assertTrue(((NumberFieldMapper) m).ignoreMalformed())); + + if (allowsIndexTimeScript()) { + checker.registerConflictCheck("script", b -> b.field("script", "foo")); + checker.registerUpdateCheck( + b -> { + minimalMapping(b); + b.field("script", "test"); + b.field("on_script_error", "reject"); + }, + b -> { + minimalMapping(b); + b.field("script", "test"); + b.field("on_script_error", "ignore"); + }, + m -> assertThat(((NumberFieldMapper)m).onScriptError(), equalTo("ignore"))); + } } @Override @@ -228,15 +249,24 @@ public void testOutOfRangeValues() throws IOException { } - public void testScriptableTypes() { + @Override + @SuppressWarnings("unchecked") + protected T compileScript(Script script, ScriptContext context) { + if (context == LongFieldScript.CONTEXT) { + return (T) LongFieldScript.PARSE_FROM_SOURCE; + } + if (context == DoubleFieldScript.CONTEXT) { + return (T) DoubleFieldScript.PARSE_FROM_SOURCE; + } + throw new UnsupportedOperationException("Unknown script " + script.getIdOrCode()); + } + + public void testScriptableTypes() throws IOException { if (allowsIndexTimeScript()) { - // won't actually compile because we don't have painless in unit tests, but we can - // check that it gets as far as trying to compile it - Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { + createDocumentMapper(fieldMapping(b -> { minimalMapping(b); b.field("script", "foo"); - }))); - assertEquals("Failed to parse mapping: script_lang not supported [painless]", e.getMessage()); + })); } else { Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> { minimalMapping(b); From fb879ef3afed94ef191107cd3f5ea182d7403f67 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Tue, 23 Mar 2021 12:35:00 +0000 Subject: [PATCH 30/39] Add some basic docs --- docs/reference/mapping/types/numeric.asciidoc | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/docs/reference/mapping/types/numeric.asciidoc b/docs/reference/mapping/types/numeric.asciidoc index c03554ec142f0..5bb33354b7c89 100644 --- a/docs/reference/mapping/types/numeric.asciidoc +++ b/docs/reference/mapping/types/numeric.asciidoc @@ -117,6 +117,7 @@ The following parameters are accepted by numeric types: Try to convert strings to numbers and truncate fractions for integers. Accepts `true` (default) and `false`. Not applicable for `unsigned_long`. + Note that this cannot be set if the `script` parameter is used. <>:: @@ -127,7 +128,8 @@ The following parameters are accepted by numeric types: <>:: If `true`, malformed numbers are ignored. If `false` (default), malformed - numbers throw an exception and reject the whole document. + numbers throw an exception and reject the whole document. Note that this + cannot be set if the `script` parameter is used. <>:: @@ -137,7 +139,25 @@ The following parameters are accepted by numeric types: Accepts a numeric value of the same `type` as the field which is substituted for any explicit `null` values. Defaults to `null`, which - means the field is treated as missing. + means the field is treated as missing. Note that this cannot be set + if the `script` parameter is used. + +`on_script_error`:: + + Defines what to do if the script defined by the `script` parameter + throws an error at indexing time. Accepts `reject` (default), which + will cause the entire document to be rejected, and `ignore`, which + will register the field in the document's + <> metadata field and continue + indexing. This parameter can only be set if the `script` field is + also set. + +`script`:: + + Configures the field to index values generated by this script, rather + than reading the values directly from the source. Scripts are in the + same format as their <>. + Scripts can only be configured on `long` and `double` field types. <>:: From d76a88b36369d435322091b1380c899b099b7566 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 24 Mar 2021 10:40:36 +0000 Subject: [PATCH 31/39] Implement document() on lazy reader properly --- .../index/mapper/PostParsePhase.java | 63 ++++++++++++++----- .../search/lookup/LeafStoredFieldsLookup.java | 8 +-- .../index/mapper/MapperTestCase.java | 56 +++++++++++++++++ 3 files changed, 107 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java index 156d7ff8f49d3..42974c473986f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.bytes.BytesReference; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashSet; @@ -195,7 +196,35 @@ public FieldInfos getFieldInfos() { @Override public void document(int docID, StoredFieldVisitor visitor) throws IOException { - visitor.binaryField(SOURCE_FIELD_INFO, sourceBytes.toBytesRef().bytes); + List fields = document.getFields().stream() + .filter(f -> f.fieldType().stored()) + .collect(Collectors.toList()); + for (IndexableField field : fields) { + FieldInfo fieldInfo = fieldInfo(field.name()); + if (visitor.needsField(fieldInfo) != StoredFieldVisitor.Status.YES) { + continue; + } + if (field.numericValue() != null) { + Number v = field.numericValue(); + if (v instanceof Integer) { + visitor.intField(fieldInfo, v.intValue()); + } else if (v instanceof Long) { + visitor.longField(fieldInfo, v.longValue()); + } else if (v instanceof Float) { + visitor.floatField(fieldInfo, v.floatValue()); + } else if (v instanceof Double) { + visitor.doubleField(fieldInfo, v.doubleValue()); + } + } else if (field.stringValue() != null) { + visitor.stringField(fieldInfo, field.stringValue().getBytes(StandardCharsets.UTF_8)); + } else if (field.binaryValue() != null) { + // We can't just pass field.binaryValue().bytes here as there may be offset/length + // considerations + byte[] data = new byte[field.binaryValue().length]; + System.arraycopy(field.binaryValue().bytes, field.binaryValue().offset, data, 0, data.length); + visitor.binaryField(fieldInfo, data); + } + } } @Override @@ -259,21 +288,23 @@ public CacheHelper getReaderCacheHelper() { } } - private static final FieldInfo SOURCE_FIELD_INFO = new FieldInfo( - "_source", - 0, - false, - false, - false, - IndexOptions.NONE, - DocValuesType.NONE, - -1, - Collections.emptyMap(), - 0, - 0, - 0, - false - ); + private static FieldInfo fieldInfo(String name) { + return new FieldInfo( + name, + 0, + false, + false, + false, + IndexOptions.NONE, + DocValuesType.NONE, + -1, + Collections.emptyMap(), + 0, + 0, + 0, + false + ); + } private static NumericDocValues numericDocValues(List values) { if (values.size() == 0) { diff --git a/server/src/main/java/org/elasticsearch/search/lookup/LeafStoredFieldsLookup.java b/server/src/main/java/org/elasticsearch/search/lookup/LeafStoredFieldsLookup.java index f4ffacf1a027b..960559638605c 100644 --- a/server/src/main/java/org/elasticsearch/search/lookup/LeafStoredFieldsLookup.java +++ b/server/src/main/java/org/elasticsearch/search/lookup/LeafStoredFieldsLookup.java @@ -25,7 +25,7 @@ import static java.util.Collections.singletonMap; @SuppressWarnings({"unchecked", "rawtypes"}) -public class LeafStoredFieldsLookup implements Map { +public class LeafStoredFieldsLookup implements Map { private final Function fieldTypeLookup; private final CheckedBiConsumer reader; @@ -49,7 +49,7 @@ public void setDocument(int docId) { } @Override - public Object get(Object key) { + public FieldLookup get(Object key) { return loadFieldData(key.toString()); } @@ -89,12 +89,12 @@ public Set entrySet() { } @Override - public Object put(Object key, Object value) { + public FieldLookup put(Object key, FieldLookup value) { throw new UnsupportedOperationException(); } @Override - public Object remove(Object key) { + public FieldLookup remove(Object key) { throw new UnsupportedOperationException(); } diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index 4ba779a243219..7f5fde5df2d69 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -33,6 +33,7 @@ import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.search.DocValueFormat; +import org.elasticsearch.search.lookup.LeafStoredFieldsLookup; import org.elasticsearch.search.lookup.SearchLookup; import org.elasticsearch.search.lookup.SourceLookup; @@ -547,4 +548,59 @@ public void onError(PostParseContext context, Exception e) throws IOException { postParsePhase.execute(); }); } + + public final void testIndexTimeStoredFieldsAccess() throws IOException { + + MapperService mapperService; + try { + mapperService = createMapperService(fieldMapping(b -> { + minimalMapping(b); + b.field("store", true); + })); + assertParseMinimalWarnings(); + } catch (MapperParsingException e) { + assertParseMinimalWarnings(); + assumeFalse("Field type does not support stored fields", true); + return; + } + + MappedFieldType fieldType = mapperService.fieldType("field"); + SourceToParse source = source(this::writeField); + ParsedDocument doc = mapperService.documentMapper().parse(source); + + withLuceneIndex(mapperService, iw -> iw.addDocument(doc.rootDoc()), ir -> { + + LeafReaderContext ctx = ir.leaves().get(0); + SearchLookup lookup = new SearchLookup(f -> fieldType, (f, s) -> { throw new UnsupportedOperationException(); }); + LeafStoredFieldsLookup storedFields = lookup.getLeafSearchLookup(ctx).fields(); + storedFields.setDocument(0); + + PostParseExecutor indexTimeExecutor = new PostParseExecutor() { + @Override + public void execute(PostParseContext context) { + SearchLookup indexLookup = new SearchLookup(f -> fieldType, (f, s) -> { throw new UnsupportedOperationException(); }); + LeafStoredFieldsLookup indexStoredFields = lookup.getLeafSearchLookup(context.leafReaderContext).fields(); + indexStoredFields.setDocument(0); + + // compare index and search time stored fields + assertThat(storedFields.get("field").getValues(), equalTo(indexStoredFields.get("field").getValues())); + } + + @Override + public void onError(PostParseContext context, Exception e) throws IOException { + throw new IOException(e); + } + }; + + ParseContext pc = mock(ParseContext.class); + when(pc.rootDoc()).thenReturn(doc.rootDoc()); + when(pc.sourceToParse()).thenReturn(source); + + PostParsePhase postParsePhase = new PostParsePhase( + Map.of("test", indexTimeExecutor), + mapperService::fieldType, + pc); + postParsePhase.execute(); + }); + } } From d20db3a9391194ea83b89a1fd2200a929c97baa3 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 29 Mar 2021 12:11:56 +0100 Subject: [PATCH 32/39] feedback --- docs/reference/mapping/types/numeric.asciidoc | 9 +-- .../index/mapper/DocumentParser.java | 2 +- .../index/mapper/FieldMapper.java | 2 +- .../index/mapper/NumberFieldMapper.java | 18 ++--- .../index/mapper/PostParseContext.java | 2 +- .../index/mapper/PostParseExecutor.java | 2 - .../index/mapper/PostParsePhase.java | 23 +++---- .../index/mapper/MapperTestCase.java | 69 ++++++++----------- 8 files changed, 53 insertions(+), 74 deletions(-) diff --git a/docs/reference/mapping/types/numeric.asciidoc b/docs/reference/mapping/types/numeric.asciidoc index 5bb33354b7c89..b0377eeec9a1e 100644 --- a/docs/reference/mapping/types/numeric.asciidoc +++ b/docs/reference/mapping/types/numeric.asciidoc @@ -154,10 +154,11 @@ The following parameters are accepted by numeric types: `script`:: - Configures the field to index values generated by this script, rather - than reading the values directly from the source. Scripts are in the - same format as their <>. - Scripts can only be configured on `long` and `double` field types. + If this parameter is set, then the field will index values generated + by this script, rather than reading the values directly from the + source. Scripts are in the same format as their + <>. Scripts can only be + configured on `long` and `double` field types. <>:: diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index b46f2b0f77180..5a12c9de7bd69 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -108,7 +108,7 @@ private static void internalParseDocument(MappingLookup lookup, MetadataFieldMap parseObjectOrNested(context, root); } - PostParsePhase.executePostParsePhases(lookup, context); + PostParsePhase.executePostParsePhases(context); for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) { metadataMapper.postParse(context); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index 4ace04c884e9a..f98fe4410f452 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -879,7 +879,7 @@ public static FieldMapper.Parameter> scriptParam( } Script script = Script.parse(o); if (script.getType() == ScriptType.STORED) { - throw new IllegalArgumentException("stored scripts are not supported on scripted field [" + n + "]"); + throw new IllegalArgumentException("stored scripts are not supported on field [" + n + "]"); } return compiler.apply(script, c.scriptCompiler()); }, diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index c92af5b141c8a..e88d22293b38f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -384,9 +384,9 @@ public Double parse(XContentParser parser, boolean coerce) throws IOException { @Override public BiFunction> compiler(String fieldName) { - return (script, service) -> new MapperScript<>(script) { + return (script, compiler) -> new MapperScript<>(script) { - final DoubleFieldScript.Factory scriptFactory = service.compile(script, DoubleFieldScript.CONTEXT); + final DoubleFieldScript.Factory scriptFactory = compiler.compile(script, DoubleFieldScript.CONTEXT); @Override public void executeAndEmit(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { @@ -728,9 +728,9 @@ public Long parse(XContentParser parser, boolean coerce) throws IOException { @Override public BiFunction> compiler(String fieldName) { - return (script, service) -> new MapperScript<>(script) { + return (script, compiler) -> new MapperScript<>(script) { - final LongFieldScript.Factory scriptFactory = service.compile(script, LongFieldScript.CONTEXT); + final LongFieldScript.Factory scriptFactory = compiler.compile(script, LongFieldScript.CONTEXT); @Override public void executeAndEmit(SearchLookup lookup, LeafReaderContext ctx, int doc, Consumer emitter) { @@ -1245,18 +1245,14 @@ public PostParseExecutor getPostParseExecutor() { if (this.script == null) { return null; } - return new PostParseExecutor() { - @Override - public void execute(PostParseContext context) { + return context -> { + try { script.executeAndEmit( context.searchLookup, context.leafReaderContext, 0, v -> indexValue(context.pc, v)); - } - - @Override - public void onError(PostParseContext context, Exception e) throws IOException { + } catch (Exception e) { if ("ignore".equals(onScriptError)) { context.pc.addIgnoredField(name()); } else { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java index 1cd02be151133..44360902c31c5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java @@ -18,7 +18,7 @@ /** * Holds state useful for post-parse document processing */ -public class PostParseContext { +public final class PostParseContext { /** A search lookup over the parsed document */ public final SearchLookup searchLookup; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java index 5dd804d06ec53..83005d3d91c45 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java @@ -17,6 +17,4 @@ public interface PostParseExecutor { void execute(PostParseContext context) throws IOException; - void onError(PostParseContext context, Exception e) throws IOException; - } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java index 42974c473986f..f46d02445016f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java @@ -26,7 +26,6 @@ import org.apache.lucene.index.Terms; import org.apache.lucene.util.Bits; import org.apache.lucene.util.BytesRef; -import org.elasticsearch.common.bytes.BytesReference; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -50,11 +49,10 @@ public class PostParsePhase { /** * Given a mapping, collects all {@link PostParseExecutor}s and executes them - * @param lookup the MappingLookup to collect executors from * @param parseContext the ParseContext of the current document */ - public static void executePostParsePhases(MappingLookup lookup, ParseContext parseContext) throws IOException { - PostParsePhase postParsePhase = lookup.buildPostParsePhase(parseContext); + public static void executePostParsePhases(ParseContext parseContext) throws IOException { + PostParsePhase postParsePhase = parseContext.mappingLookup().buildPostParsePhase(parseContext); if (postParsePhase == null) { return; } @@ -67,7 +65,6 @@ public static void executePostParsePhases(MappingLookup lookup, ParseContext par ParseContext pc) { LazyDocumentReader reader = new LazyDocumentReader( pc.rootDoc(), - pc.sourceToParse().source(), postParseExecutors.keySet()); this.context = new PostParseContext(fieldTypeLookup, pc, reader.getContext()); postParseExecutors.forEach((k, c) -> fieldExecutors.put(k, new OneTimeFieldExecutor(c))); @@ -93,11 +90,7 @@ private class OneTimeFieldExecutor { void execute() throws IOException { if (executed == false) { - try { - executor.execute(context); - } catch (Exception e) { - executor.onError(context, e); - } + executor.execute(context); executed = true; } } @@ -106,13 +99,11 @@ void execute() throws IOException { private class LazyDocumentReader extends LeafReader { private final ParseContext.Document document; - private final BytesReference sourceBytes; private final Set calculatedFields; private final Set fieldPath = new LinkedHashSet<>(); - private LazyDocumentReader(ParseContext.Document document, BytesReference sourceBytes, Set calculatedFields) { + private LazyDocumentReader(ParseContext.Document document, Set calculatedFields) { this.document = document; - this.sourceBytes = sourceBytes; this.calculatedFields = calculatedFields; } @@ -121,7 +112,9 @@ private void checkField(String field) throws IOException { // this means that a mapper script is referring to another calculated field; // in which case we need to execute that field first. We also check for loops here if (fieldPath.add(field) == false) { - throw new IllegalStateException("Loop in field resolution detected: " + String.join("->", fieldPath) + "->" + field); + throw new IllegalArgumentException( + "Loop in field resolution detected: " + String.join("->", fieldPath) + "->" + field + ); } assert fieldExecutors.containsKey(field); fieldExecutors.get(field).execute(); @@ -288,6 +281,8 @@ public CacheHelper getReaderCacheHelper() { } } + // Our StoredFieldsVisitor implementations only check the name of the passed-in + // FieldInfo, so that's the only value we need to set here. private static FieldInfo fieldInfo(String name) { return new FieldInfo( name, diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index 64a7c84de1168..21c723992731c 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -588,6 +588,10 @@ protected boolean supportsScripts() { return true; } + /** + * Checks that field data from this field produces the same values for query-time + * scripts and for index-time scripts + */ public final void testIndexTimeFieldData() throws IOException { assumeTrue("Field type does not support scripting", supportsScripts()); MapperService mapperService = createMapperService(fieldMapping(this::minimalMapping)); @@ -611,26 +615,18 @@ public final void testIndexTimeFieldData() throws IOException { fieldData.setNextDocId(0); - PostParseExecutor indexTimeExecutor = new PostParseExecutor() { - @Override - public void execute(PostParseContext context) throws IOException { - ScriptDocValues indexData = fieldType - .fielddataBuilder("test", () -> { - throw new UnsupportedOperationException(); - }) - .build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService()) - .load(context.leafReaderContext) - .getScriptValues(); - indexData.setNextDocId(0); - - // compare index and search time fielddata - assertThat(fieldData, equalTo(indexData)); - } - - @Override - public void onError(PostParseContext context, Exception e) throws IOException { - throw new IOException(e); - } + PostParseExecutor indexTimeExecutor = context -> { + ScriptDocValues indexData = fieldType + .fielddataBuilder("test", () -> { + throw new UnsupportedOperationException(); + }) + .build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService()) + .load(context.leafReaderContext) + .getScriptValues(); + indexData.setNextDocId(0); + + // compare index and search time fielddata + assertThat(fieldData, equalTo(indexData)); }; ParseContext pc = mock(ParseContext.class); @@ -645,6 +641,10 @@ public void onError(PostParseContext context, Exception e) throws IOException { }); } + /** + * Checks that loading stored fields for this field produces the same set of values + * for query time scripts and index time scripts + */ public final void testIndexTimeStoredFieldsAccess() throws IOException { MapperService mapperService; @@ -664,37 +664,26 @@ public final void testIndexTimeStoredFieldsAccess() throws IOException { SourceToParse source = source(this::writeField); ParsedDocument doc = mapperService.documentMapper().parse(source); + SearchLookup lookup = new SearchLookup(f -> fieldType, (f, s) -> { + throw new UnsupportedOperationException(); + }); + withLuceneIndex(mapperService, iw -> iw.addDocument(doc.rootDoc()), ir -> { LeafReaderContext ctx = ir.leaves().get(0); - SearchLookup lookup = new SearchLookup(f -> fieldType, (f, s) -> { - throw new UnsupportedOperationException(); - }); LeafStoredFieldsLookup storedFields = lookup.getLeafSearchLookup(ctx).fields(); storedFields.setDocument(0); - PostParseExecutor indexTimeExecutor = new PostParseExecutor() { - @Override - public void execute(PostParseContext context) { - SearchLookup indexLookup = new SearchLookup(f -> fieldType, (f, s) -> { - throw new UnsupportedOperationException(); - }); - LeafStoredFieldsLookup indexStoredFields = lookup.getLeafSearchLookup(context.leafReaderContext).fields(); - indexStoredFields.setDocument(0); - - // compare index and search time stored fields - assertThat(storedFields.get("field").getValues(), equalTo(indexStoredFields.get("field").getValues())); - } + PostParseExecutor indexTimeExecutor = context -> { + LeafStoredFieldsLookup indexStoredFields = lookup.getLeafSearchLookup(context.leafReaderContext).fields(); + indexStoredFields.setDocument(0); - @Override - public void onError(PostParseContext context, Exception e) throws IOException { - throw new IOException(e); - } + // compare index and search time stored fields + assertThat(storedFields.get("field").getValues(), equalTo(indexStoredFields.get("field").getValues())); }; ParseContext pc = mock(ParseContext.class); when(pc.rootDoc()).thenReturn(doc.rootDoc()); - when(pc.sourceToParse()).thenReturn(source); PostParsePhase postParsePhase = new PostParsePhase( Map.of("test", indexTimeExecutor), From 9b6043625ce229b3b2bbce1d8e9fbe01cef12742 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 29 Mar 2021 16:55:03 +0100 Subject: [PATCH 33/39] DocumentLeafReader to top; execute functions to DocumentParser; rename to IndexTimeScript --- .../index/mapper/DocumentLeafReader.java | 474 ++++++++++++++++ .../index/mapper/DocumentParser.java | 57 +- .../index/mapper/FieldMapper.java | 4 +- ...arseExecutor.java => IndexTimeScript.java} | 7 +- .../index/mapper/MappingLookup.java | 22 +- .../index/mapper/NumberFieldMapper.java | 14 +- .../index/mapper/PostParseContext.java | 43 -- .../index/mapper/PostParsePhase.java | 528 ------------------ .../index/mapper/CalculatedFieldTests.java | 12 +- .../index/mapper/MapperTestCase.java | 36 +- 10 files changed, 571 insertions(+), 626 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/DocumentLeafReader.java rename server/src/main/java/org/elasticsearch/index/mapper/{PostParseExecutor.java => IndexTimeScript.java} (66%) delete mode 100644 server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java delete mode 100644 server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentLeafReader.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentLeafReader.java new file mode 100644 index 0000000000000..74bea6de6a563 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentLeafReader.java @@ -0,0 +1,474 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.index.mapper; + +import org.apache.lucene.index.BinaryDocValues; +import org.apache.lucene.index.DocValuesType; +import org.apache.lucene.index.FieldInfo; +import org.apache.lucene.index.FieldInfos; +import org.apache.lucene.index.Fields; +import org.apache.lucene.index.IndexOptions; +import org.apache.lucene.index.IndexableField; +import org.apache.lucene.index.LeafMetaData; +import org.apache.lucene.index.LeafReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.PointValues; +import org.apache.lucene.index.SortedDocValues; +import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.index.SortedSetDocValues; +import org.apache.lucene.index.StoredFieldVisitor; +import org.apache.lucene.index.Terms; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.function.Consumer; +import java.util.stream.Collectors; + +// A LeafReader over a lucene document that exposes doc values and stored fields. +// Note that unlike lucene's MemoryIndex implementation, this holds no state and +// does not attempt to do any analysis on text fields. It also supports stored +// fields where MemoryIndex does not. It is used to back index-time scripts that +// reference field data and stored fields from a document that has not yet been +// indexed. +class DocumentLeafReader extends LeafReader { + + private final ParseContext.Document document; + private final Map> calculatedFields; + private final Set fieldPath = new LinkedHashSet<>(); + + DocumentLeafReader(ParseContext.Document document, Map> calculatedFields) { + this.document = document; + this.calculatedFields = calculatedFields; + } + + private void checkField(String field) { + if (calculatedFields.containsKey(field)) { + // this means that a mapper script is referring to another calculated field; + // in which case we need to execute that field first. We also check for loops here + if (fieldPath.add(field) == false) { + throw new IllegalArgumentException( + "Loop in field resolution detected: " + String.join("->", fieldPath) + "->" + field + ); + } + calculatedFields.get(field).accept(this.getContext()); + fieldPath.remove(field); + } + } + + @Override + public NumericDocValues getNumericDocValues(String field) throws IOException { + checkField(field); + List values = document.getFields().stream() + .filter(f -> Objects.equals(f.name(), field)) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.NUMERIC) + .map(IndexableField::numericValue) + .sorted() + .collect(Collectors.toList()); + return numericDocValues(values); + } + + @Override + public BinaryDocValues getBinaryDocValues(String field) throws IOException { + checkField(field); + List values = document.getFields().stream() + .filter(f -> Objects.equals(f.name(), field)) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.BINARY) + .map(IndexableField::binaryValue) + .sorted() + .collect(Collectors.toList()); + return binaryDocValues(values); + } + + @Override + public SortedDocValues getSortedDocValues(String field) throws IOException { + checkField(field); + List values = document.getFields().stream() + .filter(f -> Objects.equals(f.name(), field)) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.SORTED) + .map(IndexableField::binaryValue) + .sorted() + .collect(Collectors.toList()); + return sortedDocValues(values); + } + + @Override + public SortedNumericDocValues getSortedNumericDocValues(String field) throws IOException { + checkField(field); + List values = document.getFields().stream() + .filter(f -> Objects.equals(f.name(), field)) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.SORTED_NUMERIC) + .map(IndexableField::numericValue) + .sorted() + .collect(Collectors.toList()); + return sortedNumericDocValues(values); + } + + @Override + public SortedSetDocValues getSortedSetDocValues(String field) throws IOException { + List values = document.getFields().stream() + .filter(f -> Objects.equals(f.name(), field)) + .filter(f -> f.fieldType().docValuesType() == DocValuesType.SORTED_SET) + .map(IndexableField::binaryValue) + .sorted() + .collect(Collectors.toList()); + return sortedSetDocValues(values); + } + + @Override + public FieldInfos getFieldInfos() { + return new FieldInfos(new FieldInfo[0]); + } + + @Override + public void document(int docID, StoredFieldVisitor visitor) throws IOException { + List fields = document.getFields().stream() + .filter(f -> f.fieldType().stored()) + .collect(Collectors.toList()); + for (IndexableField field : fields) { + FieldInfo fieldInfo = fieldInfo(field.name()); + if (visitor.needsField(fieldInfo) != StoredFieldVisitor.Status.YES) { + continue; + } + if (field.numericValue() != null) { + Number v = field.numericValue(); + if (v instanceof Integer) { + visitor.intField(fieldInfo, v.intValue()); + } else if (v instanceof Long) { + visitor.longField(fieldInfo, v.longValue()); + } else if (v instanceof Float) { + visitor.floatField(fieldInfo, v.floatValue()); + } else if (v instanceof Double) { + visitor.doubleField(fieldInfo, v.doubleValue()); + } + } else if (field.stringValue() != null) { + visitor.stringField(fieldInfo, field.stringValue().getBytes(StandardCharsets.UTF_8)); + } else if (field.binaryValue() != null) { + // We can't just pass field.binaryValue().bytes here as there may be offset/length + // considerations + byte[] data = new byte[field.binaryValue().length]; + System.arraycopy(field.binaryValue().bytes, field.binaryValue().offset, data, 0, data.length); + visitor.binaryField(fieldInfo, data); + } + } + } + + @Override + public CacheHelper getCoreCacheHelper() { + throw new UnsupportedOperationException(); + } + + @Override + public Terms terms(String field) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public NumericDocValues getNormValues(String field) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public Bits getLiveDocs() { + throw new UnsupportedOperationException(); + } + + @Override + public PointValues getPointValues(String field) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public void checkIntegrity() throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public LeafMetaData getMetaData() { + throw new UnsupportedOperationException(); + } + + @Override + public Fields getTermVectors(int docID) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public int numDocs() { + throw new UnsupportedOperationException(); + } + + @Override + public int maxDoc() { + throw new UnsupportedOperationException(); + } + + @Override + protected void doClose() throws IOException { + throw new UnsupportedOperationException(); + } + + @Override + public CacheHelper getReaderCacheHelper() { + throw new UnsupportedOperationException(); + } + + // Our StoredFieldsVisitor implementations only check the name of the passed-in + // FieldInfo, so that's the only value we need to set here. + private static FieldInfo fieldInfo(String name) { + return new FieldInfo( + name, + 0, + false, + false, + false, + IndexOptions.NONE, + DocValuesType.NONE, + -1, + Collections.emptyMap(), + 0, + 0, + 0, + false + ); + } + + private static NumericDocValues numericDocValues(List values) { + if (values.size() == 0) { + return null; + } + return new NumericDocValues() { + @Override + public long longValue() { + return values.get(0).longValue(); + } + + @Override + public boolean advanceExact(int target) { + return true; + } + + @Override + public int docID() { + return 0; + } + + @Override + public int nextDoc() { + return 0; + } + + @Override + public int advance(int target) { + return 0; + } + + @Override + public long cost() { + return 0; + } + }; + } + + private static SortedNumericDocValues sortedNumericDocValues(List values) { + if (values.size() == 0) { + return null; + } + return new SortedNumericDocValues() { + + int i = -1; + + @Override + public long nextValue() { + i++; + return values.get(i).longValue(); + } + + @Override + public int docValueCount() { + return values.size(); + } + + @Override + public boolean advanceExact(int target) { + return true; + } + + @Override + public int docID() { + return 0; + } + + @Override + public int nextDoc() { + return 0; + } + + @Override + public int advance(int target) { + return 0; + } + + @Override + public long cost() { + return 0; + } + }; + } + + private static BinaryDocValues binaryDocValues(List values) { + if (values.size() == 0) { + return null; + } + return new BinaryDocValues() { + @Override + public BytesRef binaryValue() { + return values.get(0); + } + + @Override + public boolean advanceExact(int target) { + return true; + } + + @Override + public int docID() { + return 0; + } + + @Override + public int nextDoc() { + return 0; + } + + @Override + public int advance(int target) { + return 0; + } + + @Override + public long cost() { + return 0; + } + }; + } + + private static SortedDocValues sortedDocValues(List values) { + if (values.size() == 0) { + return null; + } + return new SortedDocValues() { + + @Override + public int ordValue() { + return 0; + } + + @Override + public BytesRef lookupOrd(int ord) { + return values.get(0); + } + + @Override + public int getValueCount() { + return values.size(); + } + + @Override + public boolean advanceExact(int target) { + return true; + } + + @Override + public int docID() { + return 0; + } + + @Override + public int nextDoc() { + return 0; + } + + @Override + public int advance(int target) { + return 0; + } + + @Override + public long cost() { + return 0; + } + }; + } + + private static SortedSetDocValues sortedSetDocValues(List values) { + if (values.size() == 0) { + return null; + } + return new SortedSetDocValues() { + + int i = -1; + + @Override + public long nextOrd() { + i++; + if (i >= values.size()) { + return NO_MORE_ORDS; + } + return i; + } + + @Override + public BytesRef lookupOrd(long ord) { + return values.get((int)ord); + } + + @Override + public long getValueCount() { + return values.size(); + } + + @Override + public boolean advanceExact(int target) { + i = -1; + return true; + } + + @Override + public int docID() { + return 0; + } + + @Override + public int nextDoc() { + i = -1; + return 0; + } + + @Override + public int advance(int target) { + i = -1; + return 0; + } + + @Override + public long cost() { + return 0; + } + }; + } +} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index ff36e33ef835d..4488d58dcfb9c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -10,6 +10,7 @@ import org.apache.lucene.document.Field; import org.apache.lucene.index.IndexableField; +import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.Query; import org.elasticsearch.Version; import org.elasticsearch.common.Strings; @@ -21,14 +22,20 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.fielddata.IndexFieldDataCache; import org.elasticsearch.index.query.SearchExecutionContext; +import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; +import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.function.Consumer; import java.util.function.Function; /** A parser for documents, given mappings from a DocumentMapper */ @@ -108,13 +115,61 @@ private static void internalParseDocument(MappingLookup lookup, MetadataFieldMap parseObjectOrNested(context, root); } - PostParsePhase.executePostParsePhases(context); + executeIndexTimeScripts(context); for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) { metadataMapper.postParse(context); } } + private static void executeIndexTimeScripts(ParseContext context) { + Map indexTimeScripts = context.mappingLookup().indexTimeScripts(); + if (indexTimeScripts.isEmpty()) { + return; + } + SearchLookup searchLookup = new SearchLookup( + context.mappingLookup().indexTimeLookup()::get, + (ft, s) -> ft.fielddataBuilder(context.indexSettings().getIndex().getName(), s).build( + new IndexFieldDataCache.None(), + new NoneCircuitBreakerService()) + ); + Map> oneTimeScripts = new HashMap<>(); + indexTimeScripts.forEach((k, c) -> { + OneTimeFieldExecutor executor = new OneTimeFieldExecutor(c, searchLookup, context); + oneTimeScripts.put(k, executor::execute); + }); + DocumentLeafReader reader = new DocumentLeafReader(context.rootDoc(), oneTimeScripts); + + for (Consumer script : oneTimeScripts.values()) { + script.accept(reader.getContext()); + } + } + + // field scripts can be called both by executeIndexTimeScripts() and via the document reader, + // so to ensure that we don't run field scripts multiple times we guard them with + // this one-time executor class + private static class OneTimeFieldExecutor { + + IndexTimeScript executor; + boolean executed = false; + final SearchLookup searchLookup; + final ParseContext parseContext; + + OneTimeFieldExecutor(IndexTimeScript executor, SearchLookup searchLookup, ParseContext parseContext) { + this.executor = executor; + this.searchLookup = searchLookup; + this.parseContext = parseContext; + } + + void execute(LeafReaderContext ctx) { + if (executed == false) { + executor.execute(searchLookup, ctx, parseContext); + executed = true; + } + } + } + + private static void validateStart(XContentParser parser) throws IOException { // will result in START_OBJECT XContentParser.Token token = parser.nextToken(); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index f98fe4410f452..8a5266bf47e4a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -173,9 +173,9 @@ public void parse(ParseContext context) throws IOException { } /** - * Returns a post-parse executor for this field mapper, or {@code null} if none is defined + * Returns an index time script for this field mapper, or {@code null} if none is defined */ - public PostParseExecutor getPostParseExecutor() { + public IndexTimeScript getIndexTimeScript() { return null; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java b/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScript.java similarity index 66% rename from server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java rename to server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScript.java index 83005d3d91c45..164177261f589 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParseExecutor.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IndexTimeScript.java @@ -8,13 +8,14 @@ package org.elasticsearch.index.mapper; -import java.io.IOException; +import org.apache.lucene.index.LeafReaderContext; +import org.elasticsearch.search.lookup.SearchLookup; /** * Runs after a document has been parsed */ -public interface PostParseExecutor { +public interface IndexTimeScript { - void execute(PostParseContext context) throws IOException; + void execute(SearchLookup searchLookup, LeafReaderContext ctx, ParseContext pc); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java index aa8ec2ef802d6..152a74857ed5e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -56,7 +56,7 @@ private CacheKey() {} private final FieldTypeLookup fieldTypeLookup; private final FieldTypeLookup indexTimeLookup; private final Map indexAnalyzersMap = new HashMap<>(); - private final Map postParsePhases = new HashMap<>(); + private final Map indexTimeScripts = new HashMap<>(); private final DocumentParser documentParser; private final Mapping mapping; private final IndexSettings indexSettings; @@ -139,9 +139,9 @@ public MappingLookup(Mapping mapping, throw new MapperParsingException("Field [" + mapper.name() + "] is defined more than once"); } indexAnalyzersMap.putAll(mapper.indexAnalyzers()); - PostParseExecutor postParsePhase = mapper.getPostParseExecutor(); - if (postParsePhase != null) { - postParsePhases.put(mapper.fieldType().name(), postParsePhase); + IndexTimeScript indexTimeScript = mapper.getIndexTimeScript(); + if (indexTimeScript != null) { + indexTimeScripts.put(mapper.fieldType().name(), indexTimeScript); } } @@ -155,7 +155,7 @@ public MappingLookup(Mapping mapping, } this.fieldTypeLookup = new FieldTypeLookup(mappers, aliasMappers, mapping.getRoot().runtimeFields()); - this.indexTimeLookup = postParsePhases.isEmpty() ? null : new FieldTypeLookup(mappers, aliasMappers, Collections.emptyList()); + this.indexTimeLookup = indexTimeScripts.isEmpty() ? null : new FieldTypeLookup(mappers, aliasMappers, Collections.emptyList()); this.fieldMappers = Collections.unmodifiableMap(fieldMappers); this.objectMappers = Collections.unmodifiableMap(objects); } @@ -174,12 +174,12 @@ FieldTypeLookup fieldTypesLookup() { return fieldTypeLookup; } - PostParsePhase buildPostParsePhase(ParseContext pc) { - if (postParsePhases.isEmpty()) { - return null; - } - assert indexTimeLookup != null; - return new PostParsePhase(postParsePhases, indexTimeLookup::get, pc); + FieldTypeLookup indexTimeLookup() { + return indexTimeLookup; + } + + Map indexTimeScripts() { + return indexTimeScripts; } public NamedAnalyzer indexAnalyzer(String field, Function unmappedFieldAnalyzer) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 744aa29a347c2..127a1b62eb7b5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -1241,22 +1241,18 @@ private void indexValue(ParseContext context, Number numericValue) { } @Override - public PostParseExecutor getPostParseExecutor() { + public IndexTimeScript getIndexTimeScript() { if (this.script == null) { return null; } - return context -> { + return (lookup, ctx, pc) -> { try { - script.executeAndEmit( - context.searchLookup, - context.leafReaderContext, - 0, - v -> indexValue(context.pc, v)); + script.executeAndEmit(lookup, ctx, 0, v -> indexValue(pc, v)); } catch (Exception e) { if ("ignore".equals(onScriptError)) { - context.pc.addIgnoredField(name()); + pc.addIgnoredField(name()); } else { - throw new IOException("Error executing script on field [" + name() + "]", e); + throw new MapperParsingException("Error executing script on field [" + name() + "]", e); } } }; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java deleted file mode 100644 index 44360902c31c5..0000000000000 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParseContext.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.index.mapper; - -import org.apache.lucene.index.LeafReaderContext; -import org.elasticsearch.index.fielddata.IndexFieldDataCache; -import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; -import org.elasticsearch.search.lookup.SearchLookup; - -import java.util.function.Function; - -/** - * Holds state useful for post-parse document processing - */ -public final class PostParseContext { - - /** A search lookup over the parsed document */ - public final SearchLookup searchLookup; - - /** A LeafReaderContext for a lucene reader based on the parsed document */ - public final LeafReaderContext leafReaderContext; - - /** The ParseContext used during document parsing */ - public final ParseContext pc; - - PostParseContext(Function fieldTypeLookup, ParseContext pc, LeafReaderContext ctx) { - this.searchLookup = new SearchLookup( - fieldTypeLookup, - (ft, s) -> ft.fielddataBuilder(pc.indexSettings().getIndex().getName(), s).build( - new IndexFieldDataCache.None(), - new NoneCircuitBreakerService()) - ); - this.pc = pc; - this.leafReaderContext = ctx; - } - -} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java b/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java deleted file mode 100644 index f46d02445016f..0000000000000 --- a/server/src/main/java/org/elasticsearch/index/mapper/PostParsePhase.java +++ /dev/null @@ -1,528 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.index.mapper; - -import org.apache.lucene.index.BinaryDocValues; -import org.apache.lucene.index.DocValuesType; -import org.apache.lucene.index.FieldInfo; -import org.apache.lucene.index.FieldInfos; -import org.apache.lucene.index.Fields; -import org.apache.lucene.index.IndexOptions; -import org.apache.lucene.index.IndexableField; -import org.apache.lucene.index.LeafMetaData; -import org.apache.lucene.index.LeafReader; -import org.apache.lucene.index.NumericDocValues; -import org.apache.lucene.index.PointValues; -import org.apache.lucene.index.SortedDocValues; -import org.apache.lucene.index.SortedNumericDocValues; -import org.apache.lucene.index.SortedSetDocValues; -import org.apache.lucene.index.StoredFieldVisitor; -import org.apache.lucene.index.Terms; -import org.apache.lucene.util.Bits; -import org.apache.lucene.util.BytesRef; - -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.Collections; -import java.util.HashMap; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Set; -import java.util.function.Function; -import java.util.stream.Collectors; - -/** - * Executes post parse phases on mappings - */ -public class PostParsePhase { - - private final Map fieldExecutors = new HashMap<>(); - private final PostParseContext context; - - /** - * Given a mapping, collects all {@link PostParseExecutor}s and executes them - * @param parseContext the ParseContext of the current document - */ - public static void executePostParsePhases(ParseContext parseContext) throws IOException { - PostParsePhase postParsePhase = parseContext.mappingLookup().buildPostParsePhase(parseContext); - if (postParsePhase == null) { - return; - } - postParsePhase.execute(); - } - - PostParsePhase( - Map postParseExecutors, - Function fieldTypeLookup, - ParseContext pc) { - LazyDocumentReader reader = new LazyDocumentReader( - pc.rootDoc(), - postParseExecutors.keySet()); - this.context = new PostParseContext(fieldTypeLookup, pc, reader.getContext()); - postParseExecutors.forEach((k, c) -> fieldExecutors.put(k, new OneTimeFieldExecutor(c))); - } - - void execute() throws IOException { - for (OneTimeFieldExecutor executor : fieldExecutors.values()) { - executor.execute(); - } - } - - // FieldExecutors can be called both by executePostParse() and from the lazy reader, - // so to ensure that we don't run field scripts multiple times we guard them with - // this one-time executor class - private class OneTimeFieldExecutor { - - PostParseExecutor executor; - boolean executed = false; - - OneTimeFieldExecutor(PostParseExecutor executor) { - this.executor = executor; - } - - void execute() throws IOException { - if (executed == false) { - executor.execute(context); - executed = true; - } - } - } - - private class LazyDocumentReader extends LeafReader { - - private final ParseContext.Document document; - private final Set calculatedFields; - private final Set fieldPath = new LinkedHashSet<>(); - - private LazyDocumentReader(ParseContext.Document document, Set calculatedFields) { - this.document = document; - this.calculatedFields = calculatedFields; - } - - private void checkField(String field) throws IOException { - if (calculatedFields.contains(field)) { - // this means that a mapper script is referring to another calculated field; - // in which case we need to execute that field first. We also check for loops here - if (fieldPath.add(field) == false) { - throw new IllegalArgumentException( - "Loop in field resolution detected: " + String.join("->", fieldPath) + "->" + field - ); - } - assert fieldExecutors.containsKey(field); - fieldExecutors.get(field).execute(); - calculatedFields.remove(field); - fieldPath.remove(field); - } - } - - @Override - public NumericDocValues getNumericDocValues(String field) throws IOException { - checkField(field); - List values = document.getFields().stream() - .filter(f -> Objects.equals(f.name(), field)) - .filter(f -> f.fieldType().docValuesType() == DocValuesType.NUMERIC) - .map(IndexableField::numericValue) - .sorted() - .collect(Collectors.toList()); - return numericDocValues(values); - } - - @Override - public BinaryDocValues getBinaryDocValues(String field) throws IOException { - checkField(field); - List values = document.getFields().stream() - .filter(f -> Objects.equals(f.name(), field)) - .filter(f -> f.fieldType().docValuesType() == DocValuesType.BINARY) - .map(IndexableField::binaryValue) - .sorted() - .collect(Collectors.toList()); - return binaryDocValues(values); - } - - @Override - public SortedDocValues getSortedDocValues(String field) throws IOException { - checkField(field); - List values = document.getFields().stream() - .filter(f -> Objects.equals(f.name(), field)) - .filter(f -> f.fieldType().docValuesType() == DocValuesType.SORTED) - .map(IndexableField::binaryValue) - .sorted() - .collect(Collectors.toList()); - return sortedDocValues(values); - } - - @Override - public SortedNumericDocValues getSortedNumericDocValues(String field) throws IOException { - checkField(field); - List values = document.getFields().stream() - .filter(f -> Objects.equals(f.name(), field)) - .filter(f -> f.fieldType().docValuesType() == DocValuesType.SORTED_NUMERIC) - .map(IndexableField::numericValue) - .sorted() - .collect(Collectors.toList()); - return sortedNumericDocValues(values); - } - - @Override - public SortedSetDocValues getSortedSetDocValues(String field) throws IOException { - List values = document.getFields().stream() - .filter(f -> Objects.equals(f.name(), field)) - .filter(f -> f.fieldType().docValuesType() == DocValuesType.SORTED_SET) - .map(IndexableField::binaryValue) - .sorted() - .collect(Collectors.toList()); - return sortedSetDocValues(values); - } - - @Override - public FieldInfos getFieldInfos() { - return new FieldInfos(new FieldInfo[0]); - } - - @Override - public void document(int docID, StoredFieldVisitor visitor) throws IOException { - List fields = document.getFields().stream() - .filter(f -> f.fieldType().stored()) - .collect(Collectors.toList()); - for (IndexableField field : fields) { - FieldInfo fieldInfo = fieldInfo(field.name()); - if (visitor.needsField(fieldInfo) != StoredFieldVisitor.Status.YES) { - continue; - } - if (field.numericValue() != null) { - Number v = field.numericValue(); - if (v instanceof Integer) { - visitor.intField(fieldInfo, v.intValue()); - } else if (v instanceof Long) { - visitor.longField(fieldInfo, v.longValue()); - } else if (v instanceof Float) { - visitor.floatField(fieldInfo, v.floatValue()); - } else if (v instanceof Double) { - visitor.doubleField(fieldInfo, v.doubleValue()); - } - } else if (field.stringValue() != null) { - visitor.stringField(fieldInfo, field.stringValue().getBytes(StandardCharsets.UTF_8)); - } else if (field.binaryValue() != null) { - // We can't just pass field.binaryValue().bytes here as there may be offset/length - // considerations - byte[] data = new byte[field.binaryValue().length]; - System.arraycopy(field.binaryValue().bytes, field.binaryValue().offset, data, 0, data.length); - visitor.binaryField(fieldInfo, data); - } - } - } - - @Override - public CacheHelper getCoreCacheHelper() { - throw new UnsupportedOperationException(); - } - - @Override - public Terms terms(String field) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public NumericDocValues getNormValues(String field) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public Bits getLiveDocs() { - throw new UnsupportedOperationException(); - } - - @Override - public PointValues getPointValues(String field) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public void checkIntegrity() throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public LeafMetaData getMetaData() { - throw new UnsupportedOperationException(); - } - - @Override - public Fields getTermVectors(int docID) throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public int numDocs() { - throw new UnsupportedOperationException(); - } - - @Override - public int maxDoc() { - throw new UnsupportedOperationException(); - } - - @Override - protected void doClose() throws IOException { - throw new UnsupportedOperationException(); - } - - @Override - public CacheHelper getReaderCacheHelper() { - throw new UnsupportedOperationException(); - } - } - - // Our StoredFieldsVisitor implementations only check the name of the passed-in - // FieldInfo, so that's the only value we need to set here. - private static FieldInfo fieldInfo(String name) { - return new FieldInfo( - name, - 0, - false, - false, - false, - IndexOptions.NONE, - DocValuesType.NONE, - -1, - Collections.emptyMap(), - 0, - 0, - 0, - false - ); - } - - private static NumericDocValues numericDocValues(List values) { - if (values.size() == 0) { - return null; - } - return new NumericDocValues() { - @Override - public long longValue() { - return values.get(0).longValue(); - } - - @Override - public boolean advanceExact(int target) { - return true; - } - - @Override - public int docID() { - return 0; - } - - @Override - public int nextDoc() { - return 0; - } - - @Override - public int advance(int target) { - return 0; - } - - @Override - public long cost() { - return 0; - } - }; - } - - private static SortedNumericDocValues sortedNumericDocValues(List values) { - if (values.size() == 0) { - return null; - } - return new SortedNumericDocValues() { - - int i = -1; - - @Override - public long nextValue() { - i++; - return values.get(i).longValue(); - } - - @Override - public int docValueCount() { - return values.size(); - } - - @Override - public boolean advanceExact(int target) { - return true; - } - - @Override - public int docID() { - return 0; - } - - @Override - public int nextDoc() { - return 0; - } - - @Override - public int advance(int target) { - return 0; - } - - @Override - public long cost() { - return 0; - } - }; - } - - private static BinaryDocValues binaryDocValues(List values) { - if (values.size() == 0) { - return null; - } - return new BinaryDocValues() { - @Override - public BytesRef binaryValue() { - return values.get(0); - } - - @Override - public boolean advanceExact(int target) { - return true; - } - - @Override - public int docID() { - return 0; - } - - @Override - public int nextDoc() { - return 0; - } - - @Override - public int advance(int target) { - return 0; - } - - @Override - public long cost() { - return 0; - } - }; - } - - private static SortedDocValues sortedDocValues(List values) { - if (values.size() == 0) { - return null; - } - return new SortedDocValues() { - - @Override - public int ordValue() { - return 0; - } - - @Override - public BytesRef lookupOrd(int ord) { - return values.get(0); - } - - @Override - public int getValueCount() { - return values.size(); - } - - @Override - public boolean advanceExact(int target) { - return true; - } - - @Override - public int docID() { - return 0; - } - - @Override - public int nextDoc() { - return 0; - } - - @Override - public int advance(int target) { - return 0; - } - - @Override - public long cost() { - return 0; - } - }; - } - - private static SortedSetDocValues sortedSetDocValues(List values) { - if (values.size() == 0) { - return null; - } - return new SortedSetDocValues() { - - int i = -1; - - @Override - public long nextOrd() { - i++; - if (i >= values.size()) { - return NO_MORE_ORDS; - } - return i; - } - - @Override - public BytesRef lookupOrd(long ord) { - return values.get((int)ord); - } - - @Override - public long getValueCount() { - return values.size(); - } - - @Override - public boolean advanceExact(int target) { - i = -1; - return true; - } - - @Override - public int docID() { - return 0; - } - - @Override - public int nextDoc() { - i = -1; - return 0; - } - - @Override - public int advance(int target) { - i = -1; - return 0; - } - - @Override - public long cost() { - return 0; - } - }; - } -} diff --git a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java index bb4f63c0667de..a6090c43df475 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/CalculatedFieldTests.java @@ -133,8 +133,7 @@ public void testLoopDetection() throws IOException { })); Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> {}))); - assertEquals("failed to parse", e.getMessage()); - assertEquals("Error executing script on field [field1]", e.getCause().getMessage()); + assertEquals("Error executing script on field [field1]", e.getMessage()); Throwable root = e.getCause(); while (root.getCause() != null) { @@ -165,8 +164,8 @@ public void testCannotReferToRuntimeFields() throws IOException { })); Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> {}))); - assertEquals("Error executing script on field [index-field]", e.getCause().getMessage()); - assertEquals("No field found for [runtime-field] in mapping", e.getCause().getCause().getMessage()); + assertEquals("Error executing script on field [index-field]", e.getMessage()); + assertEquals("No field found for [runtime-field] in mapping", e.getCause().getMessage()); } public void testScriptErrorParameterRequiresScript() { @@ -220,9 +219,8 @@ public void testRejectScriptErrors() throws IOException { })); Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.field("message", "foo")))); - assertThat(e.getMessage(), equalTo("failed to parse")); - assertThat(e.getCause().getMessage(), equalTo("Error executing script on field [message_error]")); - assertThat(e.getCause().getCause().getMessage(), equalTo("Oops!")); + assertThat(e.getMessage(), equalTo("Error executing script on field [message_error]")); + assertThat(e.getCause().getMessage(), equalTo("Oops!")); } @Override diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index 21c723992731c..a5a617309e941 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -38,6 +38,7 @@ import org.elasticsearch.search.lookup.SourceLookup; import java.io.IOException; +import java.io.UncheckedIOException; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -615,29 +616,26 @@ public final void testIndexTimeFieldData() throws IOException { fieldData.setNextDocId(0); - PostParseExecutor indexTimeExecutor = context -> { + IndexTimeScript indexTimeExecutor = (lookup, context, pc) -> { ScriptDocValues indexData = fieldType .fielddataBuilder("test", () -> { throw new UnsupportedOperationException(); }) .build(new IndexFieldDataCache.None(), new NoneCircuitBreakerService()) - .load(context.leafReaderContext) + .load(context) .getScriptValues(); - indexData.setNextDocId(0); + try { + indexData.setNextDocId(0); + } catch (IOException e) { + throw new UncheckedIOException(e); + } // compare index and search time fielddata assertThat(fieldData, equalTo(indexData)); }; - ParseContext pc = mock(ParseContext.class); - when(pc.rootDoc()).thenReturn(doc.rootDoc()); - when(pc.sourceToParse()).thenReturn(source); - - PostParsePhase postParsePhase = new PostParsePhase( - Map.of("test", indexTimeExecutor), - mapperService::fieldType, - pc); - postParsePhase.execute(); + DocumentLeafReader reader = new DocumentLeafReader(doc.rootDoc(), Collections.emptyMap()); + indexTimeExecutor.execute(null, reader.getContext(), null); }); } @@ -674,22 +672,16 @@ public final void testIndexTimeStoredFieldsAccess() throws IOException { LeafStoredFieldsLookup storedFields = lookup.getLeafSearchLookup(ctx).fields(); storedFields.setDocument(0); - PostParseExecutor indexTimeExecutor = context -> { - LeafStoredFieldsLookup indexStoredFields = lookup.getLeafSearchLookup(context.leafReaderContext).fields(); + IndexTimeScript indexTimeExecutor = (l, context, pc) -> { + LeafStoredFieldsLookup indexStoredFields = lookup.getLeafSearchLookup(context).fields(); indexStoredFields.setDocument(0); // compare index and search time stored fields assertThat(storedFields.get("field").getValues(), equalTo(indexStoredFields.get("field").getValues())); }; - ParseContext pc = mock(ParseContext.class); - when(pc.rootDoc()).thenReturn(doc.rootDoc()); - - PostParsePhase postParsePhase = new PostParsePhase( - Map.of("test", indexTimeExecutor), - mapperService::fieldType, - pc); - postParsePhase.execute(); + DocumentLeafReader reader = new DocumentLeafReader(doc.rootDoc(), Collections.emptyMap()); + indexTimeExecutor.execute(null, reader.getContext(), null); }); } From c888b32143f1fb5379a8a36930ee0a7ff3b1deed Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Mon, 29 Mar 2021 18:04:36 +0100 Subject: [PATCH 34/39] Remove MapperScript --- .../index/mapper/DynamicFieldsBuilder.java | 14 +++- .../index/mapper/FieldMapper.java | 11 +-- .../index/mapper/MapperScript.java | 64 ----------------- .../index/mapper/NumberFieldMapper.java | 72 +++++++++++-------- .../fielddata/AbstractFieldDataTestCase.java | 12 ++-- .../fielddata/IndexFieldDataServiceTests.java | 22 +++--- .../index/mapper/NumberFieldTypeTests.java | 6 +- 7 files changed, 78 insertions(+), 123 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/index/mapper/MapperScript.java diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java b/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java index 2419219ed70dc..9c678449fcc38 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java @@ -271,7 +271,12 @@ public void newDynamicStringField(ParseContext context, String name) throws IOEx @Override public void newDynamicLongField(ParseContext context, String name) throws IOException { createDynamicField( - new NumberFieldMapper.Builder(name, NumberFieldMapper.NumberType.LONG, context.indexSettings().getSettings()), context); + new NumberFieldMapper.Builder( + name, + NumberFieldMapper.NumberType.LONG, + null, + context.indexSettings().getSettings() + ), context); } @Override @@ -279,8 +284,11 @@ public void newDynamicDoubleField(ParseContext context, String name) throws IOEx // no templates are defined, we use float by default instead of double // since this is much more space-efficient and should be enough most of // the time - createDynamicField(new NumberFieldMapper.Builder(name, - NumberFieldMapper.NumberType.FLOAT, context.indexSettings().getSettings()), context); + createDynamicField(new NumberFieldMapper.Builder( + name, + NumberFieldMapper.NumberType.FLOAT, + null, + context.indexSettings().getSettings()), context); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index 8a5266bf47e4a..6092bb3d79705 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -26,8 +26,6 @@ import org.elasticsearch.index.mapper.FieldNamesFieldMapper.FieldNamesFieldType; import org.elasticsearch.index.mapper.Mapper.TypeParser.ParserContext; import org.elasticsearch.script.Script; -import org.elasticsearch.script.ScriptCompiler; -import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.ScriptType; import java.io.IOException; @@ -860,14 +858,11 @@ public static Parameter docValuesParam(Function i /** * Defines a script parameter - * @param compiler a function that produces a compiled script, given a {@link Script} and {@link ScriptService} * @param initializer retrieves the equivalent parameter from an existing FieldMapper for use in merges - * @param the type of the compiled script * @return a script parameter */ - public static FieldMapper.Parameter> scriptParam( - BiFunction> compiler, - Function>initializer + public static FieldMapper.Parameter