Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Mapper.updateFieldType() #56986

Merged
merged 9 commits into from
May 26, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -647,16 +647,6 @@ public SearchAsYouTypeFieldMapper(String simpleName,
this.maxShingleSize = maxShingleSize;
}

@Override
public FieldMapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldType) {
SearchAsYouTypeFieldMapper fieldMapper = (SearchAsYouTypeFieldMapper) super.updateFieldType(fullNameToFieldType);
fieldMapper.prefixField = (PrefixFieldMapper) fieldMapper.prefixField.updateFieldType(fullNameToFieldType);
for (int i = 0; i < fieldMapper.shingleFields.length; i++) {
fieldMapper.shingleFields[i] = (ShingleFieldMapper) fieldMapper.shingleFields[i].updateFieldType(fullNameToFieldType);
}
return fieldMapper;
}

@Override
protected void parseCreateField(ParseContext context) throws IOException {
final String value = context.externalValueSet() ? context.externalValue().toString() : context.parser().textOrNull();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,21 +59,25 @@ static class Defaults {
}

static class Builder extends FieldMapper.Builder<Builder> {
Builder() {

final String joinField;

Builder(String joinField) {
super(NAME, Defaults.FIELD_TYPE, Defaults.FIELD_TYPE);
builder = this;
this.joinField = joinField;
}

@Override
public MetaJoinFieldMapper build(BuilderContext context) {
fieldType.setName(NAME);
return new MetaJoinFieldMapper(name, fieldType, context.indexSettings());
return new MetaJoinFieldMapper(name, joinField, (MetaJoinFieldType) fieldType, context.indexSettings());
}
}

public static class MetaJoinFieldType extends StringFieldType {

private ParentJoinFieldMapper mapper;
private String joinField;

MetaJoinFieldType() {}

Expand Down Expand Up @@ -110,8 +114,12 @@ public Object valueForDisplay(Object value) {
return binaryValue.utf8ToString();
}

public ParentJoinFieldMapper getMapper() {
return mapper;
public void setJoinField(String joinField) {
this.joinField = joinField;
}

public String getJoinField() {
return joinField;
}

@Override
Expand All @@ -120,12 +128,10 @@ public Query existsQuery(QueryShardContext context) {
}
}

MetaJoinFieldMapper(String name, MappedFieldType fieldType, Settings indexSettings) {
MetaJoinFieldMapper(String name, String joinField, MetaJoinFieldType fieldType, Settings indexSettings) {
super(name, fieldType, ParentIdFieldMapper.Defaults.FIELD_TYPE, indexSettings, MultiFields.empty(), CopyTo.empty());
}
fieldType.setJoinField(joinField);

void setFieldMapper(ParentJoinFieldMapper mapper) {
fieldType().mapper = mapper;
}

@Override
Expand All @@ -140,7 +146,6 @@ protected MetaJoinFieldMapper clone() {

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

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.fielddata.plain.SortedSetOrdinalsIndexFieldData;
import org.elasticsearch.index.mapper.ContentPath;
import org.elasticsearch.index.mapper.DocumentFieldMappers;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.Mapper;
Expand Down Expand Up @@ -85,7 +87,13 @@ public static class Defaults {
public static ParentJoinFieldMapper getMapper(MapperService service) {
MetaJoinFieldMapper.MetaJoinFieldType fieldType =
(MetaJoinFieldMapper.MetaJoinFieldType) service.fieldType(MetaJoinFieldMapper.NAME);
return fieldType == null ? null : fieldType.getMapper();
if (fieldType == null) {
return null;
}
DocumentMapper mapper = service.documentMapper();
String joinField = fieldType.getJoinField();
DocumentFieldMappers fieldMappers = mapper.mappers();
return (ParentJoinFieldMapper) fieldMappers.getMapper(joinField);
}

private static String getParentIdFieldName(String joinFieldName, String parentName) {
Expand Down Expand Up @@ -160,7 +168,7 @@ public ParentJoinFieldMapper build(BuilderContext context) {
})
.forEach(parentIdFields::add);
checkParentFields(name(), parentIdFields);
MetaJoinFieldMapper unique = new MetaJoinFieldMapper.Builder().build(context);
MetaJoinFieldMapper unique = new MetaJoinFieldMapper.Builder(name).build(context);
return new ParentJoinFieldMapper(name, fieldType, context.indexSettings(),
unique, Collections.unmodifiableList(parentIdFields), eagerGlobalOrdinals);
}
Expand Down Expand Up @@ -262,7 +270,6 @@ protected ParentJoinFieldMapper(String simpleName,
super(simpleName, fieldType, Defaults.FIELD_TYPE, indexSettings, MultiFields.empty(), CopyTo.empty());
this.parentIdFields = parentIdFields;
this.uniqueFieldMapper = uniqueFieldMapper;
this.uniqueFieldMapper.setFieldMapper(this);
this.eagerGlobalOrdinals = eagerGlobalOrdinals;
}

Expand Down Expand Up @@ -353,20 +360,6 @@ protected void mergeOptions(FieldMapper other, List<String> conflicts) {
this.eagerGlobalOrdinals = joinMergeWith.eagerGlobalOrdinals;
this.parentIdFields = Collections.unmodifiableList(newParentIdFields);
this.uniqueFieldMapper = (MetaJoinFieldMapper) uniqueFieldMapper.merge(joinMergeWith.uniqueFieldMapper);
uniqueFieldMapper.setFieldMapper(this);
}

@Override
public FieldMapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldType) {
ParentJoinFieldMapper fieldMapper = (ParentJoinFieldMapper) super.updateFieldType(fullNameToFieldType);
final List<ParentIdFieldMapper> newMappers = new ArrayList<> ();
for (ParentIdFieldMapper mapper : fieldMapper.parentIdFields) {
newMappers.add((ParentIdFieldMapper) mapper.updateFieldType(fullNameToFieldType));
}
fieldMapper.parentIdFields = Collections.unmodifiableList(newMappers);
this.uniqueFieldMapper = (MetaJoinFieldMapper) uniqueFieldMapper.updateFieldType(fullNameToFieldType);
uniqueFieldMapper.setFieldMapper(this);
return fieldMapper;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.mapper.ContentPath;
import org.elasticsearch.index.mapper.DocumentFieldMappers;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.IdFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.Mapper;
Expand Down Expand Up @@ -278,8 +280,13 @@ protected MapperService mapperServiceMock() {
ParentJoinFieldMapper joinFieldMapper = createJoinFieldMapper();
MapperService mapperService = mock(MapperService.class);
MetaJoinFieldMapper.MetaJoinFieldType metaJoinFieldType = mock(MetaJoinFieldMapper.MetaJoinFieldType.class);
when(metaJoinFieldType.getMapper()).thenReturn(joinFieldMapper);
when(metaJoinFieldType.getJoinField()).thenReturn("join_field");
when(mapperService.fieldType("_parent_join")).thenReturn(metaJoinFieldType);
DocumentFieldMappers fieldMappers = new DocumentFieldMappers(Collections.singleton(joinFieldMapper),
Collections.emptyList(), null, null, null);
DocumentMapper mockMapper = mock(DocumentMapper.class);
when(mockMapper.mappers()).thenReturn(fieldMappers);
when(mapperService.documentMapper()).thenReturn(mockMapper);
return mapperService;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.mapper.ContentPath;
import org.elasticsearch.index.mapper.DocumentFieldMappers;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.IdFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.Mapper;
Expand Down Expand Up @@ -166,8 +168,13 @@ protected MapperService mapperServiceMock() {
ParentJoinFieldMapper joinFieldMapper = createJoinFieldMapper();
MapperService mapperService = mock(MapperService.class);
MetaJoinFieldMapper.MetaJoinFieldType metaJoinFieldType = mock(MetaJoinFieldMapper.MetaJoinFieldType.class);
when(metaJoinFieldType.getMapper()).thenReturn(joinFieldMapper);
when(metaJoinFieldType.getJoinField()).thenReturn("join_field");
when(mapperService.fieldType("_parent_join")).thenReturn(metaJoinFieldType);
DocumentFieldMappers fieldMappers = new DocumentFieldMappers(Collections.singleton(joinFieldMapper),
Collections.emptyList(), null, null, null);
DocumentMapper mockMapper = mock(DocumentMapper.class);
when(mockMapper.mappers()).thenReturn(fieldMappers);
when(mapperService.documentMapper()).thenReturn(mockMapper);
return mapperService;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,11 @@ public void testUpdateRelations() throws Exception {
.endObject()
.endObject()
.endObject().endObject());
docMapper = indexService.mapperService().merge("type", new CompressedXContent(updateMapping),
docMapper = indexService.mapperService().merge("_doc", new CompressedXContent(updateMapping),
MapperService.MergeReason.MAPPING_UPDATE);
assertTrue(docMapper.mappers().getMapper("join_field") == ParentJoinFieldMapper.getMapper(indexService.mapperService()));
ParentJoinFieldMapper mapper = ParentJoinFieldMapper.getMapper(indexService.mapperService());
assertNotNull(mapper);
assertEquals("join_field", mapper.name());
assertTrue(mapper.hasChild("child2"));
assertFalse(mapper.hasParent("child2"));
assertTrue(mapper.hasChild("grand_child2"));
Expand All @@ -322,10 +323,11 @@ public void testUpdateRelations() throws Exception {
.endObject()
.endObject()
.endObject().endObject());
docMapper = indexService.mapperService().merge("type", new CompressedXContent(updateMapping),
docMapper = indexService.mapperService().merge("_doc", new CompressedXContent(updateMapping),
MapperService.MergeReason.MAPPING_UPDATE);
assertTrue(docMapper.mappers().getMapper("join_field") == ParentJoinFieldMapper.getMapper(indexService.mapperService()));
ParentJoinFieldMapper mapper = ParentJoinFieldMapper.getMapper(indexService.mapperService());
assertNotNull(mapper);
assertEquals("join_field", mapper.name());
assertTrue(mapper.hasParent("other"));
assertFalse(mapper.hasChild("other"));
assertTrue(mapper.hasChild("child_other1"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -359,30 +359,6 @@ Tuple<List<BytesRef>, Map<String, List<byte[]>>> extractTermsAndRanges(IndexRead
this.rangeFieldMapper = rangeFieldMapper;
}

@Override
public FieldMapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldType) {
PercolatorFieldMapper updated = (PercolatorFieldMapper) super.updateFieldType(fullNameToFieldType);
KeywordFieldMapper queryTermsUpdated = (KeywordFieldMapper) queryTermsField.updateFieldType(fullNameToFieldType);
KeywordFieldMapper extractionResultUpdated = (KeywordFieldMapper) extractionResultField.updateFieldType(fullNameToFieldType);
BinaryFieldMapper queryBuilderUpdated = (BinaryFieldMapper) queryBuilderField.updateFieldType(fullNameToFieldType);
RangeFieldMapper rangeFieldMapperUpdated = (RangeFieldMapper) rangeFieldMapper.updateFieldType(fullNameToFieldType);
NumberFieldMapper msmFieldMapperUpdated = (NumberFieldMapper) minimumShouldMatchFieldMapper.updateFieldType(fullNameToFieldType);

if (updated == this && queryTermsUpdated == queryTermsField && extractionResultUpdated == extractionResultField
&& queryBuilderUpdated == queryBuilderField && rangeFieldMapperUpdated == rangeFieldMapper) {
return this;
}
if (updated == this) {
updated = (PercolatorFieldMapper) updated.clone();
}
updated.queryTermsField = queryTermsUpdated;
updated.extractionResultField = extractionResultUpdated;
updated.queryBuilderField = queryBuilderUpdated;
updated.rangeFieldMapper = rangeFieldMapperUpdated;
updated.minimumShouldMatchFieldMapper = msmFieldMapperUpdated;
return updated;
}

@Override
public void parse(ParseContext context) throws IOException {
QueryShardContext queryShardContext = this.queryShardContext.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,19 +307,6 @@ public DocumentMapper merge(Mapping mapping) {
return new DocumentMapper(mapperService, merged);
}

/**
* Recursively update sub field types.
*/
public DocumentMapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldType) {
Mapping updated = this.mapping.updateFieldType(fullNameToFieldType);
if (updated == this.mapping) {
// no change
return this;
}
assert updated == updated.updateFieldType(fullNameToFieldType) : "updateFieldType operation is not idempotent";
return new DocumentMapper(mapperService, updated);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return mapping.toXContent(builder, params);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,11 @@
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.mapper.DynamicTemplate.XContentFieldType;
import org.elasticsearch.index.mapper.KeywordFieldMapper.KeywordFieldType;
import org.elasticsearch.index.mapper.TextFieldMapper.TextFieldType;

import java.io.IOException;
import java.time.format.DateTimeParseException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;

Expand Down Expand Up @@ -619,53 +616,6 @@ private static void parseNullValue(ParseContext context, ObjectMapper parentMapp
}
}

private static Mapper.Builder<?> createBuilderFromFieldType(final ParseContext context,
MappedFieldType fieldType, String currentFieldName) {
Mapper.Builder builder = null;
if (fieldType instanceof TextFieldType) {
builder = context.root().findTemplateBuilder(context, currentFieldName, "text", XContentFieldType.STRING);
if (builder == null) {
builder = new TextFieldMapper.Builder(currentFieldName)
.addMultiField(new KeywordFieldMapper.Builder("keyword").ignoreAbove(256));
}
} else if (fieldType instanceof KeywordFieldType) {
builder = context.root().findTemplateBuilder(context, currentFieldName, "keyword", XContentFieldType.STRING);
} else {
switch (fieldType.typeName()) {
case DateFieldMapper.CONTENT_TYPE:
builder = context.root().findTemplateBuilder(context, currentFieldName, XContentFieldType.DATE);
break;
case "long":
builder = context.root().findTemplateBuilder(context, currentFieldName, "long", XContentFieldType.LONG);
break;
case "double":
builder = context.root().findTemplateBuilder(context, currentFieldName, "double", XContentFieldType.DOUBLE);
break;
case "integer":
builder = context.root().findTemplateBuilder(context, currentFieldName, "integer", XContentFieldType.LONG);
break;
case "float":
builder = context.root().findTemplateBuilder(context, currentFieldName, "float", XContentFieldType.DOUBLE);
break;
case BooleanFieldMapper.CONTENT_TYPE:
builder = context.root().findTemplateBuilder(context, currentFieldName, "boolean", XContentFieldType.BOOLEAN);
break;
default:
break;
}
}
if (builder == null) {
Mapper.TypeParser.ParserContext parserContext = context.docMapperParser().parserContext();
Mapper.TypeParser typeParser = parserContext.typeParser(fieldType.typeName());
if (typeParser == null) {
throw new MapperParsingException("Cannot generate dynamic mappings of type [" + fieldType.typeName()
+ "] for [" + currentFieldName + "]");
}
builder = typeParser.parse(currentFieldName, new HashMap<>(), parserContext);
}
return builder;
}

private static Mapper.Builder<?> newLongBuilder(String name) {
return new NumberFieldMapper.Builder(name, NumberFieldMapper.NumberType.LONG);
}
Expand Down Expand Up @@ -801,21 +751,9 @@ private static void parseDynamicValue(final ParseContext context, ObjectMapper p
if (dynamic == ObjectMapper.Dynamic.FALSE) {
return;
}
final String path = context.path().pathAsText(currentFieldName);
final Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings().getSettings(), context.path());
final MappedFieldType existingFieldType = context.mapperService().fieldType(path);
final Mapper.Builder builder;
if (existingFieldType != null) {
// create a builder of the same type
builder = createBuilderFromFieldType(context, existingFieldType, currentFieldName);
} else {
builder = createBuilderFromDynamicValue(context, token, currentFieldName);
}
final Mapper.Builder<?> builder = createBuilderFromDynamicValue(context, token, currentFieldName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can delete createBuilderFromFieldType now, since it's unused.

Mapper mapper = builder.build(builderContext);
if (existingFieldType != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm understanding correctly, we can now assume existingFieldType will always be null, since there is only one type. In that case we could remove the logic above too, including the method createBuilderFromFieldType.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

// try to not introduce a conflict
mapper = mapper.updateFieldType(Collections.singletonMap(path, existingFieldType));
}
context.addDynamicMapper(mapper);

parseObjectOrField(context, mapper);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,6 @@ public Mapper merge(Mapper mergeWith) {
return mergeWith;
}

@Override
public Mapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldType) {
return this;
}

@Override
public Iterator<Mapper> iterator() {
return Collections.emptyIterator();
Expand Down
Loading