Disable date field mapping changing (#25285)

Make date field mapping unchangeable. Closes #25271
elastic · Jul 7, 2017 · 2e5e451 · 2e5e451
1 parent f79c2cb
commit 2e5e451
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 27 deletions.
diff --git a/core/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java
@@ -54,6 +54,7 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
+import java.util.ArrayList;
 import static org.elasticsearch.index.mapper.TypeParsers.parseDateTimeFormatter;
 
 /** A {@link FieldMapper} for ip addresses. */
@@ -211,16 +212,12 @@ public String typeName() {
         @Override
         public void checkCompatibility(MappedFieldType fieldType, List<String> conflicts, boolean strict) {
             super.checkCompatibility(fieldType, conflicts, strict);
-            if (strict) {
-                DateFieldType other = (DateFieldType)fieldType;
-                if (Objects.equals(dateTimeFormatter().format(), other.dateTimeFormatter().format()) == false) {
-                    conflicts.add("mapper [" + name()
-                        + "] is used by multiple types. Set update_all_types to true to update [format] across all types.");
-                }
-                if (Objects.equals(dateTimeFormatter().locale(), other.dateTimeFormatter().locale()) == false) {
-                    conflicts.add("mapper [" + name()
-                        + "] is used by multiple types. Set update_all_types to true to update [locale] across all types.");
-                }
+            DateFieldType other = (DateFieldType) fieldType;
+            if (Objects.equals(dateTimeFormatter().format(), other.dateTimeFormatter().format()) == false) {
+                conflicts.add("mapper [" + name() + "] has different [format] values");
+            }
+            if (Objects.equals(dateTimeFormatter().locale(), other.dateTimeFormatter().locale()) == false) {
+                conflicts.add("mapper [" + name() + "] has different [locale] values");
             }
         }
 
@@ -490,8 +487,8 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
 
     @Override
     protected void doMerge(Mapper mergeWith, boolean updateAllTypes) {
+        final DateFieldMapper other = (DateFieldMapper) mergeWith;
         super.doMerge(mergeWith, updateAllTypes);
-        DateFieldMapper other = (DateFieldMapper) mergeWith;
         this.includeInAll = other.includeInAll;
         if (other.ignoreMalformed.explicit()) {
             this.ignoreMalformed = other.ignoreMalformed;

diff --git a/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/ParentFieldMapper.java
@@ -296,19 +296,13 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
 
     @Override
     protected void doMerge(Mapper mergeWith, boolean updateAllTypes) {
+        ParentFieldMapper fieldMergeWith = (ParentFieldMapper) mergeWith;
         ParentFieldType currentFieldType = (ParentFieldType) fieldType.clone();
         super.doMerge(mergeWith, updateAllTypes);
-        ParentFieldMapper fieldMergeWith = (ParentFieldMapper) mergeWith;
         if (fieldMergeWith.parentType != null && Objects.equals(parentType, fieldMergeWith.parentType) == false) {
             throw new IllegalArgumentException("The _parent field's type option can't be changed: [" + parentType + "]->[" + fieldMergeWith.parentType + "]");
         }
 
-        List<String> conflicts = new ArrayList<>();
-        fieldType().checkCompatibility(fieldMergeWith.fieldType, conflicts, true);
-        if (conflicts.isEmpty() == false) {
-            throw new IllegalArgumentException("Merge conflicts: " + conflicts);
-        }
-
         if (active()) {
             fieldType = currentFieldType;
         }

diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java
@@ -37,6 +37,7 @@
 import java.util.Collection;
 
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.notNullValue;
 
 public class DateFieldMapperTests extends ESSingleNodeTestCase {
 
@@ -345,4 +346,26 @@ public void testTimeZoneParsing() throws Exception {
 
         assertEquals(randomDate.withZone(DateTimeZone.UTC).getMillis(), fields[0].numericValue().longValue());
     }
+
+    public void testMergeDate() throws IOException {
+        String initMapping = XContentFactory.jsonBuilder().startObject().startObject("movie")
+            .startObject("properties")
+            .startObject("release_date").field("type", "date").field("format", "yyyy/MM/dd").endObject()
+            .endObject().endObject().endObject().string();
+        DocumentMapper initMapper = indexService.mapperService().merge("movie", new CompressedXContent(initMapping),
+            MapperService.MergeReason.MAPPING_UPDATE, randomBoolean());
+
+        assertThat(initMapper.mappers().getMapper("release_date"), notNullValue());
+        assertFalse(initMapper.mappers().getMapper("release_date").fieldType().stored());
+
+        String updateFormatMapping = XContentFactory.jsonBuilder().startObject().startObject("movie")
+            .startObject("properties")
+            .startObject("release_date").field("type", "date").field("format", "epoch_millis").endObject()
+            .endObject().endObject().endObject().string();
+
+        Exception e = expectThrows(IllegalArgumentException.class,
+            () -> indexService.mapperService().merge("movie", new CompressedXContent(updateFormatMapping),
+                MapperService.MergeReason.MAPPING_UPDATE, randomBoolean()));
+        assertThat(e.getMessage(), containsString("[mapper [release_date] has different [format] values]"));
+    }
 }
diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java
@@ -58,13 +58,13 @@ protected MappedFieldType createDefaultFieldType() {
     @Before
     public void setupProperties() {
         setDummyNullValue(10);
-        addModifier(new Modifier("format", true) {
+        addModifier(new Modifier("format", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((DateFieldType) ft).setDateTimeFormatter(Joda.forPattern("basic_week_date", Locale.ROOT));
             }
         });
-        addModifier(new Modifier("locale", true) {
+        addModifier(new Modifier("locale", false) {
             @Override
             public void modify(MappedFieldType ft) {
                 ((DateFieldType) ft).setDateTimeFormatter(Joda.forPattern("date_optional_time", Locale.CANADA));

diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DocumentMapperMergeTests.java
@@ -25,13 +25,6 @@
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
-import org.elasticsearch.index.mapper.DocumentFieldMappers;
-import org.elasticsearch.index.mapper.DocumentMapper;
-import org.elasticsearch.index.mapper.DocumentMapperParser;
-import org.elasticsearch.index.mapper.MapperService;
-import org.elasticsearch.index.mapper.Mapping;
-import org.elasticsearch.index.mapper.ObjectMapper;
-import org.elasticsearch.index.mapper.ParsedDocument;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 
 import java.io.IOException;