Skip to content

Commit

Permalink
Add declarative parameters to FieldMappers (#58663)
Browse files Browse the repository at this point in the history
The FieldMapper infrastructure currently has a bunch of shared parameters, many of which
are only applicable to a subset of the 41 mapper implementations we ship with. Merging,
parsing and serialization of these parameters are spread around the class hierarchy, with
much repetitive boilerplate code required. It would be much easier to reason about these
things if we could declare the parameter set of each FieldMapper directly in the implementing
class, and share the parsing, merging and serialization logic instead.

This commit is a first effort at introducing a declarative parameter style. It adds a new FieldMapper
subclass, ParametrizedFieldMapper, and refactors two mappers, Boolean and Binary, to use it.
Parameters are declared on Builder classes, with the declaration including the parameter name,
whether or not it is updateable, a default value, how to parse it from mappings, and how to
extract it from another mapper at merge time. Builders have a getParameters method, which
returns a list of the declared parameters; this is then used for parsing, merging and serialization.
Merging is achieved by constructing a new Builder from the existing Mapper, and merging in
values from the merging Mapper; conflicts are all caught at this point, and if none exist then a new,
merged, Mapper can be built from the Builder. This allows all values on the Mapper to be final.

Other mappers can be gradually migrated to this new style, and once they have all been refactored
we can merge ParametrizedFieldMapper and FieldMapper entirely.
  • Loading branch information
romseygeek committed Jul 9, 2020
1 parent daa4832 commit 67a27e2
Show file tree
Hide file tree
Showing 20 changed files with 795 additions and 159 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,7 @@ static KeywordFieldMapper createExtractQueryFieldBuilder(String name, BuilderCon
}

static BinaryFieldMapper createQueryBuilderFieldBuilder(BuilderContext context) {
BinaryFieldMapper.Builder builder = new BinaryFieldMapper.Builder(QUERY_BUILDER_FIELD_NAME);
builder.docValues(true);
builder.indexOptions(IndexOptions.NONE);
builder.store(false);
BinaryFieldMapper.Builder builder = new BinaryFieldMapper.Builder(QUERY_BUILDER_FIELD_NAME, true);
return builder.build(context);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,16 @@ public static String nodeStringValue(Object node, String defaultValue) {
return node.toString();
}

/**
* Returns the {@link Object#toString} value of its input, or {@code null} if the input is null
*/
public static String nodeStringValue(Object node) {
if (node == null) {
return null;
}
return node.toString();
}

public static float nodeFloatValue(Object node, float defaultValue) {
if (node == null) {
return defaultValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@
package org.elasticsearch.index.mapper;

import com.carrotsearch.hppc.ObjectArrayList;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.document.StoredField;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.DocValuesFieldExistsQuery;
import org.apache.lucene.search.Query;
Expand All @@ -43,47 +41,45 @@

import java.io.IOException;
import java.time.ZoneId;
import java.util.Arrays;
import java.util.Base64;
import java.util.Collections;
import java.util.List;
import java.util.Map;

import static org.elasticsearch.index.mapper.TypeParsers.parseField;

public class BinaryFieldMapper extends FieldMapper {
public class BinaryFieldMapper extends ParametrizedFieldMapper {

public static final String CONTENT_TYPE = "binary";

public static class Defaults {
public static final FieldType FIELD_TYPE = new FieldType();

static {
FIELD_TYPE.setIndexOptions(IndexOptions.NONE);
FIELD_TYPE.setOmitNorms(true);
FIELD_TYPE.freeze();
}
private static BinaryFieldMapper toType(FieldMapper in) {
return (BinaryFieldMapper) in;
}

public static class Builder extends FieldMapper.Builder<Builder> {
public static class Builder extends ParametrizedFieldMapper.Builder {

private final Parameter<Boolean> stored = Parameter.boolParam("store", false, m -> toType(m).stored, false);
private final Parameter<Boolean> hasDocValues = Parameter.boolParam("doc_values", false, m -> toType(m).hasDocValues, false);
private final Parameter<Map<String, String>> meta
= new Parameter<>("meta", true, Collections.emptyMap(), TypeParsers::parseMeta, m -> m.fieldType().meta());

public Builder(String name) {
super(name, Defaults.FIELD_TYPE);
hasDocValues = false;
builder = this;
this(name, false);
}

public Builder(String name, boolean hasDocValues) {
super(name);
this.hasDocValues.setValue(hasDocValues);
}

@Override
public BinaryFieldMapper build(BuilderContext context) {
return new BinaryFieldMapper(name, fieldType, new BinaryFieldType(buildFullName(context), hasDocValues, meta),
multiFieldsBuilder.build(this, context), copyTo);
public List<Parameter<?>> getParameters() {
return Arrays.asList(meta, stored, hasDocValues);
}

@Override
public Builder index(boolean index) {
if (index) {
throw new MapperParsingException("Binary field [" + name() + "] cannot be indexed");
}
return builder;
public BinaryFieldMapper build(BuilderContext context) {
return new BinaryFieldMapper(name, new BinaryFieldType(buildFullName(context), hasDocValues.getValue(), meta.getValue()),
multiFieldsBuilder.build(this, context), copyTo.build(), this);
}
}

Expand All @@ -92,7 +88,7 @@ public static class TypeParser implements Mapper.TypeParser {
public BinaryFieldMapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext)
throws MapperParsingException {
BinaryFieldMapper.Builder builder = new BinaryFieldMapper.Builder(name);
parseField(builder, name, node, parserContext);
builder.parse(name, parserContext, node);
return builder;
}
}
Expand Down Expand Up @@ -167,14 +163,19 @@ public Query termQuery(Object value, QueryShardContext context) {
}
}

protected BinaryFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
MultiFields multiFields, CopyTo copyTo) {
super(simpleName, fieldType, mappedFieldType, multiFields, copyTo);
private final boolean stored;
private final boolean hasDocValues;

protected BinaryFieldMapper(String simpleName, MappedFieldType mappedFieldType,
MultiFields multiFields, CopyTo copyTo, Builder builder) {
super(simpleName, mappedFieldType, multiFields, copyTo);
this.stored = builder.stored.getValue();
this.hasDocValues = builder.hasDocValues.getValue();
}

@Override
protected void parseCreateField(ParseContext context) throws IOException {
if (!fieldType.stored() && !fieldType().hasDocValues()) {
if (stored == false && hasDocValues == false) {
return;
}
byte[] value = context.parseExternalValue(byte[].class);
Expand All @@ -188,11 +189,11 @@ protected void parseCreateField(ParseContext context) throws IOException {
if (value == null) {
return;
}
if (fieldType.stored()) {
if (stored) {
context.doc().add(new StoredField(fieldType().name(), value));
}

if (fieldType().hasDocValues()) {
if (hasDocValues) {
CustomBinaryDocValuesField field = (CustomBinaryDocValuesField) context.doc().getByKey(fieldType().name());
if (field == null) {
field = new CustomBinaryDocValuesField(fieldType().name(), value);
Expand All @@ -210,18 +211,8 @@ protected void parseCreateField(ParseContext context) throws IOException {
}

@Override
protected void mergeOptions(FieldMapper other, List<String> conflicts) {

}

@Override
protected boolean docValuesByDefault() {
return false;
}

@Override
protected boolean indexedByDefault() {
return false;
public ParametrizedFieldMapper.Builder getMergeBuilder() {
return new BinaryFieldMapper.Builder(simpleName()).init(this);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.lucene.document.Field;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.document.SortedNumericDocValuesField;
import org.apache.lucene.document.StoredField;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.DocValuesFieldExistsQuery;
Expand All @@ -30,7 +31,6 @@
import org.apache.lucene.search.TermRangeQuery;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.index.fielddata.IndexFieldData;
Expand All @@ -41,17 +41,15 @@

import java.io.IOException;
import java.time.ZoneId;
import java.util.Arrays;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

import static org.elasticsearch.index.mapper.TypeParsers.parseField;

/**
* A field mapper for boolean fields.
*/
public class BooleanFieldMapper extends FieldMapper {
public class BooleanFieldMapper extends ParametrizedFieldMapper {

public static final String CONTENT_TYPE = "boolean";

Expand All @@ -71,25 +69,37 @@ public static class Values {
public static final BytesRef FALSE = new BytesRef("F");
}

public static class Builder extends FieldMapper.Builder<Builder> {
private static BooleanFieldMapper toType(FieldMapper in) {
return (BooleanFieldMapper) in;
}

public static class Builder extends ParametrizedFieldMapper.Builder {

private final Parameter<Boolean> docValues = Parameter.boolParam("doc_values", false, m -> toType(m).hasDocValues, true);
private final Parameter<Boolean> indexed = Parameter.boolParam("index", false, m -> toType(m).indexed, true);
private final Parameter<Boolean> stored = Parameter.boolParam("store", false, m -> toType(m).stored, false);

private Boolean nullValue;
private final Parameter<Boolean> nullValue
= new Parameter<>("null_value", false, null, (n, o) -> XContentMapValues.nodeBooleanValue(o), m -> toType(m).nullValue);

private final Parameter<Float> boost = Parameter.floatParam("boost", true, m -> m.fieldType().boost(), 1.0f);
private final Parameter<Map<String, String>> meta
= new Parameter<>("meta", true, Collections.emptyMap(), TypeParsers::parseMeta, m -> m.fieldType().meta());

public Builder(String name) {
super(name, Defaults.FIELD_TYPE);
this.builder = this;
super(name);
}

public Builder nullValue(Boolean nullValue) {
this.nullValue = nullValue;
return builder;
@Override
protected List<Parameter<?>> getParameters() {
return Arrays.asList(meta, boost, docValues, indexed, nullValue, stored);
}

@Override
public BooleanFieldMapper build(BuilderContext context) {
return new BooleanFieldMapper(name, fieldType,
new BooleanFieldType(buildFullName(context), indexed, hasDocValues, meta),
multiFieldsBuilder.build(this, context), copyTo, nullValue);
MappedFieldType ft = new BooleanFieldType(buildFullName(context), indexed.getValue(), docValues.getValue(), meta.getValue());
ft.setBoost(boost.getValue());
return new BooleanFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(), this);
}
}

Expand All @@ -98,19 +108,7 @@ public static class TypeParser implements Mapper.TypeParser {
public BooleanFieldMapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext)
throws MapperParsingException {
BooleanFieldMapper.Builder builder = new BooleanFieldMapper.Builder(name);
parseField(builder, name, node, parserContext);
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
Map.Entry<String, Object> entry = iterator.next();
String propName = entry.getKey();
Object propNode = entry.getValue();
if (propName.equals("null_value")) {
if (propNode == null) {
throw new MapperParsingException("Property [null_value] cannot be null.");
}
builder.nullValue(XContentMapValues.nodeBooleanValue(propNode, name + ".null_value"));
iterator.remove();
}
}
builder.parse(name, parserContext, node);
return builder;
}
}
Expand Down Expand Up @@ -217,11 +215,17 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
}

private final Boolean nullValue;

protected BooleanFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
MultiFields multiFields, CopyTo copyTo, Boolean nullValue) {
super(simpleName, fieldType, mappedFieldType, multiFields, copyTo);
this.nullValue = nullValue;
private final boolean indexed;
private final boolean hasDocValues;
private final boolean stored;

protected BooleanFieldMapper(String simpleName, MappedFieldType mappedFieldType,
MultiFields multiFields, CopyTo copyTo, Builder builder) {
super(simpleName, mappedFieldType, multiFields, copyTo);
this.nullValue = builder.nullValue.getValue();
this.stored = builder.stored.getValue();
this.indexed = builder.indexed.getValue();
this.hasDocValues = builder.docValues.getValue();
}

@Override
Expand All @@ -231,7 +235,7 @@ public BooleanFieldType fieldType() {

@Override
protected void parseCreateField(ParseContext context) throws IOException {
if (fieldType().isSearchable() == false && !fieldType.stored() && !fieldType().hasDocValues()) {
if (indexed == false && stored == false && hasDocValues == false) {
return;
}

Expand All @@ -250,31 +254,27 @@ protected void parseCreateField(ParseContext context) throws IOException {
if (value == null) {
return;
}
if (fieldType().isSearchable() || fieldType.stored()) {
context.doc().add(new Field(fieldType().name(), value ? "T" : "F", fieldType));
if (indexed) {
context.doc().add(new Field(fieldType().name(), value ? "T" : "F", Defaults.FIELD_TYPE));
}
if (stored) {
context.doc().add(new StoredField(fieldType().name(), value ? "T" : "F"));
}
if (fieldType().hasDocValues()) {
if (hasDocValues) {
context.doc().add(new SortedNumericDocValuesField(fieldType().name(), value ? 1 : 0));
} else {
createFieldNamesField(context);
}
}

@Override
protected void mergeOptions(FieldMapper other, List<String> conflicts) {
// TODO ban updating null values
public ParametrizedFieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName()).init(this);
}

@Override
protected String contentType() {
return CONTENT_TYPE;
}

@Override
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
super.doXContentBody(builder, includeDefaults, params);
if (includeDefaults || nullValue != null) {
builder.field("null_value", nullValue);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserCont
} else if (Fields.CONTEXTS.match(fieldName, LoggingDeprecationHandler.INSTANCE)) {
builder.contextMappings(ContextMappings.load(fieldNode, parserContext.indexVersionCreated()));
iterator.remove();
} else if (parseMultiField(builder, name, parserContext, fieldName, fieldNode)) {
} else if (parseMultiField(builder::addMultiField, name, parserContext, fieldName, fieldNode)) {
iterator.remove();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ public ContentPath(int offset) {
this.index = 0;
}

public ContentPath(String path) {
this.sb = new StringBuilder();
this.offset = 0;
this.index = 0;
add(path);
}

public void add(String name) {
path[index++] = name;
if (index == path.length) { // expand if needed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserCont
} else if (propName.equals("format")) {
builder.format(propNode.toString());
iterator.remove();
} else if (TypeParsers.parseMultiField(builder, name, parserContext, propName, propNode)) {
} else if (TypeParsers.parseMultiField(builder::addMultiField, name, parserContext, propName, propNode)) {
iterator.remove();
}
}
Expand Down
Loading

0 comments on commit 67a27e2

Please sign in to comment.