From a71b128738abd2e0c0fd0d7d5e31b4b6c2a91467 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 19 Jun 2015 15:41:36 -0700 Subject: [PATCH] Address PR comments and fix customFieldDataSettings to still be copied on merge --- .../org/elasticsearch/index/mapper/MappedFieldType.java | 9 ++------- .../index/mapper/core/AbstractFieldMapper.java | 6 +++++- .../index/mapper/update/UpdateMappingOnClusterTests.java | 9 ++++++--- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java b/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java index fb4a2d24305..2ef4789f049 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java @@ -232,6 +232,7 @@ public class MappedFieldType extends FieldType { public void validateCompatible(MappedFieldType other, List conflicts) { boolean indexed = indexOptions() != IndexOptions.NONE; boolean mergeWithIndexed = other.indexOptions() != IndexOptions.NONE; + // TODO: should be validating if index options go "up" (but "down" is ok) if (indexed != mergeWithIndexed || tokenized() != other.tokenized()) { conflicts.add("mapper [" + names().fullName() + "] has different index values"); } @@ -277,13 +278,7 @@ public class MappedFieldType extends FieldType { conflicts.add("mapper [" + names().fullName() + "] has different index_name"); } - if (similarity() == null) { - if (other.similarity() != null) { - conflicts.add("mapper [" + names().fullName() + "] has different similarity"); - } - } else if (other.similarity() == null) { - conflicts.add("mapper [" + names().fullName() + "] has different similarity"); - } else if (!similarity().equals(other.similarity())) { + if (Objects.equals(similarity(), other.similarity()) == false) { conflicts.add("mapper [" + names().fullName() + "] has different similarity"); } } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java index 4878e79bedf..6239e877b39 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java @@ -45,7 +45,6 @@ import org.elasticsearch.index.mapper.MergeMappingException; import org.elasticsearch.index.mapper.MergeResult; import org.elasticsearch.index.mapper.ParseContext; import org.elasticsearch.index.mapper.internal.AllFieldMapper; -import org.elasticsearch.index.mapper.internal.TimestampFieldMapper; import org.elasticsearch.index.similarity.SimilarityLookupService; import org.elasticsearch.index.similarity.SimilarityProvider; @@ -404,6 +403,11 @@ public abstract class AbstractFieldMapper implements FieldMapper { // apply changeable values this.fieldType = fieldMergeWith.fieldType().clone(); this.fieldType().freeze(); + if (fieldMergeWith.customFieldDataSettings != null) { + if (!Objects.equal(fieldMergeWith.customFieldDataSettings, this.customFieldDataSettings)) { + this.customFieldDataSettings = fieldMergeWith.customFieldDataSettings; + } + } this.copyTo = fieldMergeWith.copyTo; } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterTests.java b/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterTests.java index 890db5e3fdb..a8082fab4cf 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.mapper.update; +import com.carrotsearch.randomizedtesting.annotations.Seed; import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse; @@ -182,9 +183,11 @@ public class UpdateMappingOnClusterTests extends ElasticsearchIntegrationTest { } @Test - public void testUpdateTimestamp() throws IOException { + @Seed(value = "12345678") + public void testUpdateTimestamp() throws Exception { + boolean enabled = randomBoolean(); XContentBuilder mapping = XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("_timestamp").field("enabled", randomBoolean()).startObject("fielddata").field("loading", "lazy").field("format", "doc_values").endObject().field("store", "no").endObject() + .startObject("_timestamp").field("enabled", enabled).startObject("fielddata").field("loading", "lazy").field("format", "doc_values").endObject().field("store", "no").endObject() .endObject().endObject(); client().admin().indices().prepareCreate("test").addMapping("type", mapping).get(); GetMappingsResponse appliedMappings = client().admin().indices().prepareGetMappings("test").get(); @@ -193,7 +196,7 @@ public class UpdateMappingOnClusterTests extends ElasticsearchIntegrationTest { assertThat((String)((LinkedHashMap) timestampMapping.get("fielddata")).get("loading"), equalTo("lazy")); assertThat((String)((LinkedHashMap) timestampMapping.get("fielddata")).get("format"), equalTo("doc_values")); mapping = XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("_timestamp").field("enabled", randomBoolean()).startObject("fielddata").field("loading", "eager").field("format", "array").endObject().field("store", "no").endObject() + .startObject("_timestamp").field("enabled", enabled).startObject("fielddata").field("loading", "eager").field("format", "array").endObject().field("store", "no").endObject() .endObject().endObject(); PutMappingResponse putMappingResponse = client().admin().indices().preparePutMapping("test").setType("type").setSource(mapping).get(); appliedMappings = client().admin().indices().prepareGetMappings("test").get();