diff --git a/core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index c277cdc4728..ced3f08b229 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -370,15 +370,6 @@ public abstract class FieldMapper extends Mapper { return; } FieldMapper fieldMergeWith = (FieldMapper) mergeWith; - List subConflicts = new ArrayList<>(); // TODO: just expose list from MergeResult? - fieldType().checkTypeName(fieldMergeWith.fieldType(), subConflicts); - if (subConflicts.isEmpty() == false) { - // return early if field types don't match - assert subConflicts.size() == 1; - mergeResult.addConflict(subConflicts.get(0)); - return; - } - multiFields.merge(mergeWith, mergeResult); if (mergeResult.simulate() == false && mergeResult.hasConflicts() == false) { 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 eaaa47c3bd2..da21e599cc9 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java @@ -154,12 +154,9 @@ class FieldTypeLookup implements Iterable { MappedFieldTypeReference ref = fullNameToFieldType.get(fieldMapper.fieldType().names().fullName()); if (ref != null) { List conflicts = new ArrayList<>(); - ref.get().checkTypeName(fieldMapper.fieldType(), conflicts); - if (conflicts.isEmpty()) { // only check compat if they are the same type - final Set types = fullNameToTypes.get(fieldMapper.fieldType().names().fullName()); - boolean strict = beStrict(type, types, updateAllTypes); - ref.get().checkCompatibility(fieldMapper.fieldType(), conflicts, strict); - } + final Set types = fullNameToTypes.get(fieldMapper.fieldType().names().fullName()); + boolean strict = beStrict(type, types, updateAllTypes); + ref.get().checkCompatibility(fieldMapper.fieldType(), conflicts, strict); if (conflicts.isEmpty() == false) { throw new IllegalArgumentException("Mapper for [" + fieldMapper.fieldType().names().fullName() + "] conflicts with existing mapping in other types:\n" + conflicts.toString()); } @@ -169,12 +166,9 @@ class FieldTypeLookup implements Iterable { MappedFieldTypeReference indexNameRef = indexNameToFieldType.get(fieldMapper.fieldType().names().indexName()); if (indexNameRef != null) { List conflicts = new ArrayList<>(); - indexNameRef.get().checkTypeName(fieldMapper.fieldType(), conflicts); - if (conflicts.isEmpty()) { // only check compat if they are the same type - final Set types = indexNameToTypes.get(fieldMapper.fieldType().names().indexName()); - boolean strict = beStrict(type, types, updateAllTypes); - indexNameRef.get().checkCompatibility(fieldMapper.fieldType(), conflicts, strict); - } + final Set types = indexNameToTypes.get(fieldMapper.fieldType().names().indexName()); + boolean strict = beStrict(type, types, updateAllTypes); + indexNameRef.get().checkCompatibility(fieldMapper.fieldType(), conflicts, strict); if (conflicts.isEmpty() == false) { throw new IllegalArgumentException("Mapper for [" + fieldMapper.fieldType().names().fullName() + "] conflicts with mapping with the same index name in other types" + conflicts.toString()); } 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 2755ca1a4e8..32e749992e6 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java @@ -229,9 +229,9 @@ public abstract class MappedFieldType extends FieldType { public abstract String typeName(); /** Checks this type is the same type as other. Adds a conflict if they are different. */ - public final void checkTypeName(MappedFieldType other, List conflicts) { + private final void checkTypeName(MappedFieldType other) { if (typeName().equals(other.typeName()) == false) { - conflicts.add("mapper [" + names().fullName() + "] cannot be changed from type [" + typeName() + "] to [" + other.typeName() + "]"); + throw new IllegalArgumentException("mapper [" + names().fullName() + "] cannot be changed from type [" + typeName() + "] to [" + other.typeName() + "]"); } else if (getClass() != other.getClass()) { throw new IllegalStateException("Type names equal for class " + getClass().getSimpleName() + " and " + other.getClass().getSimpleName()); } @@ -243,6 +243,8 @@ public abstract class MappedFieldType extends FieldType { * Otherwise, only properties which must never change in an index are checked. */ public void checkCompatibility(MappedFieldType other, List conflicts, boolean strict) { + checkTypeName(other); + boolean indexed = indexOptions() != IndexOptions.NONE; boolean mergeWithIndexed = other.indexOptions() != IndexOptions.NONE; // TODO: should be validating if index options go "up" (but "down" is ok) diff --git a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java index a45348d530c..ca0cbf194d6 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeTestCase.java @@ -281,7 +281,7 @@ public abstract class FieldTypeTestCase extends ESTestCase { public void testCheckTypeName() { final MappedFieldType fieldType = createNamedDefaultFieldType(); List conflicts = new ArrayList<>(); - fieldType.checkTypeName(fieldType, conflicts); + fieldType.checkCompatibility(fieldType, conflicts, random().nextBoolean()); // no exception assertTrue(conflicts.toString(), conflicts.isEmpty()); MappedFieldType bogus = new MappedFieldType() { @@ -291,7 +291,7 @@ public abstract class FieldTypeTestCase extends ESTestCase { public String typeName() { return fieldType.typeName();} }; try { - fieldType.checkTypeName(bogus, conflicts); + fieldType.checkCompatibility(bogus, conflicts, random().nextBoolean()); fail("expected bad types exception"); } catch (IllegalStateException e) { assertTrue(e.getMessage().contains("Type names equal")); @@ -304,10 +304,13 @@ public abstract class FieldTypeTestCase extends ESTestCase { @Override public String typeName() { return "othertype";} }; - fieldType.checkTypeName(other, conflicts); - assertFalse(conflicts.isEmpty()); - assertTrue(conflicts.get(0).contains("cannot be changed from type")); - assertEquals(1, conflicts.size()); + try { + fieldType.checkCompatibility(other, conflicts, random().nextBoolean()); + fail(); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage(), e.getMessage().contains("cannot be changed from type")); + } + assertTrue(conflicts.toString(), conflicts.isEmpty()); } public void testCheckCompatibility() { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingTests.java index f0a8f5d079d..abf5f4819cd 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingTests.java @@ -151,7 +151,7 @@ public class UpdateMappingTests extends ESSingleNodeTestCase { fail(); } catch (IllegalArgumentException e) { // expected - assertTrue(e.getMessage().contains("conflicts with existing mapping in other types")); + assertTrue(e.getMessage(), e.getMessage().contains("mapper [foo] cannot be changed from type [long] to [double]")); } try { @@ -159,7 +159,7 @@ public class UpdateMappingTests extends ESSingleNodeTestCase { fail(); } catch (IllegalArgumentException e) { // expected - assertTrue(e.getMessage().contains("conflicts with existing mapping in other types")); + assertTrue(e.getMessage(), e.getMessage().contains("mapper [foo] cannot be changed from type [long] to [double]")); } assertTrue(mapperService.documentMapper("type1").mapping().root().getMapper("foo") instanceof LongFieldMapper); @@ -186,7 +186,7 @@ public class UpdateMappingTests extends ESSingleNodeTestCase { fail(); } catch (IllegalArgumentException e) { // expected - assertTrue(e.getMessage().contains("conflicts with existing mapping in other types")); + assertTrue(e.getMessage(), e.getMessage().contains("mapper [foo] cannot be changed from type [long] to [double]")); } try { @@ -194,7 +194,7 @@ public class UpdateMappingTests extends ESSingleNodeTestCase { fail(); } catch (IllegalArgumentException e) { // expected - assertTrue(e.getMessage().contains("conflicts with existing mapping in other types")); + assertTrue(e.getMessage(), e.getMessage().contains("mapper [foo] cannot be changed from type [long] to [double]")); } assertTrue(mapperService.documentMapper("type1").mapping().root().getMapper("foo") instanceof LongFieldMapper);