diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index c7ee704de4f..fbe82a70bd7 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -327,6 +327,11 @@ public class DocumentMapper implements ToXContent { */ public DocumentMapper updateFieldType(Map 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); } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java b/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java index 5f6fddf09ef..d05077fe5cf 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java @@ -93,7 +93,7 @@ class FieldTypeLookup implements Iterable { // is the update even legal? checkCompatibility(type, fieldMapper, updateAllTypes); - if (fieldType != fullNameFieldType) { + if (fieldType.equals(fullNameFieldType) == false) { fullName = fullName.copyAndPut(fieldType.name(), fieldMapper.fieldType()); } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/Mapping.java b/core/src/main/java/org/elasticsearch/index/mapper/Mapping.java index 0b92dbe4517..9cb3ea7a57e 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/Mapping.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/Mapping.java @@ -104,12 +104,22 @@ public final class Mapping implements ToXContent { * Recursively update sub field types. */ public Mapping updateFieldType(Map fullNameToFieldType) { - final MetadataFieldMapper[] updatedMeta = Arrays.copyOf(metadataMappers, metadataMappers.length); - for (int i = 0; i < updatedMeta.length; ++i) { - updatedMeta[i] = (MetadataFieldMapper) updatedMeta[i].updateFieldType(fullNameToFieldType); + MetadataFieldMapper[] updatedMeta = null; + for (int i = 0; i < metadataMappers.length; ++i) { + MetadataFieldMapper currentFieldMapper = metadataMappers[i]; + MetadataFieldMapper updatedFieldMapper = (MetadataFieldMapper) currentFieldMapper.updateFieldType(fullNameToFieldType); + if (updatedFieldMapper != currentFieldMapper) { + if (updatedMeta == null) { + updatedMeta = Arrays.copyOf(metadataMappers, metadataMappers.length); + } + updatedMeta[i] = updatedFieldMapper; + } } RootObjectMapper updatedRoot = root.updateFieldType(fullNameToFieldType); - return new Mapping(indexCreated, updatedRoot, updatedMeta, meta); + if (updatedMeta == null && updatedRoot == root) { + return this; + } + return new Mapping(indexCreated, updatedRoot, updatedMeta == null ? metadataMappers : updatedMeta, meta); } @Override diff --git a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java index 80e5df4081c..1091e313416 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java @@ -72,11 +72,12 @@ public class FieldTypeLookupTests extends ESTestCase { MockFieldMapper f = new MockFieldMapper("foo"); MockFieldMapper f2 = new MockFieldMapper("foo"); FieldTypeLookup lookup = new FieldTypeLookup(); - lookup = lookup.copyAndAddAll("type1", newList(f), randomBoolean()); - FieldTypeLookup lookup2 = lookup.copyAndAddAll("type2", newList(f2), randomBoolean()); + lookup = lookup.copyAndAddAll("type1", newList(f), true); + FieldTypeLookup lookup2 = lookup.copyAndAddAll("type2", newList(f2), true); - assertSame(f2.fieldType(), lookup2.get("foo")); assertEquals(1, size(lookup2.iterator())); + assertSame(f.fieldType(), lookup2.get("foo")); + assertEquals(f2.fieldType(), lookup2.get("foo")); } public void testAddExistingIndexName() { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java b/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java index a6270dfc953..87afdedf89d 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java @@ -19,16 +19,6 @@ package org.elasticsearch.index.mapper; -import java.io.IOException; -import java.io.UncheckedIOException; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.concurrent.ExecutionException; -import java.util.function.Function; - import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; @@ -39,8 +29,17 @@ import org.elasticsearch.index.mapper.MapperService.MergeReason; import org.elasticsearch.index.mapper.NumberFieldMapper.NumberFieldType; import org.elasticsearch.test.ESSingleNodeTestCase; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.function.Function; + import static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.Matchers.hasToString; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.startsWith; @@ -169,7 +168,6 @@ public class MapperServiceTests extends ESSingleNodeTestCase { assertThat(mapperService.unmappedFieldType("string"), instanceOf(KeywordFieldType.class)); } - public void testMergeWithMap() throws Throwable { IndexService indexService1 = createIndex("index1"); MapperService mapperService = indexService1.mapperService(); @@ -187,4 +185,34 @@ public class MapperServiceTests extends ESSingleNodeTestCase { () -> mapperService.merge(mappings, false)); assertThat(e.getMessage(), startsWith("Failed to parse mapping [type1]: ")); } + + public void testOtherDocumentMappersOnlyUpdatedWhenChangingFieldType() throws IOException { + IndexService indexService = createIndex("test"); + + CompressedXContent simpleMapping = new CompressedXContent(XContentFactory.jsonBuilder().startObject() + .startObject("properties") + .startObject("field") + .field("type", "text") + .endObject() + .endObject().endObject().bytes()); + + indexService.mapperService().merge("type1", simpleMapping, MergeReason.MAPPING_UPDATE, true); + DocumentMapper documentMapper = indexService.mapperService().documentMapper("type1"); + + indexService.mapperService().merge("type2", simpleMapping, MergeReason.MAPPING_UPDATE, true); + assertSame(indexService.mapperService().documentMapper("type1"), documentMapper); + + CompressedXContent normsDisabledMapping = new CompressedXContent(XContentFactory.jsonBuilder().startObject() + .startObject("properties") + .startObject("field") + .field("type", "text") + .startObject("norms") + .field("enabled", false) + .endObject() + .endObject() + .endObject().endObject().bytes()); + + indexService.mapperService().merge("type3", normsDisabledMapping, MergeReason.MAPPING_UPDATE, true); + assertNotSame(indexService.mapperService().documentMapper("type1"), documentMapper); + } }