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 53e875cea91..c4fec8cf095 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -336,8 +336,6 @@ public class DocumentMapper implements ToXContent { private void addMappers(Collection objectMappers, Collection fieldMappers, boolean updateAllTypes) { assert mappingLock.isWriteLockedByCurrentThread(); - // first ensure we don't have any incompatible new fields - mapperService.checkNewMappersCompatibility(objectMappers, fieldMappers, updateAllTypes); // update mappers for this document type Map builder = new HashMap<>(this.objectMappers); @@ -356,6 +354,7 @@ public class DocumentMapper implements ToXContent { public MergeResult merge(Mapping mapping, boolean simulate, boolean updateAllTypes) { try (ReleasableLock lock = mappingWriteLock.acquire()) { + mapperService.checkMappersCompatibility(type, mapping, updateAllTypes); final MergeResult mergeResult = new MergeResult(simulate, updateAllTypes); this.mapping.merge(mapping, mergeResult); if (simulate == false) { 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 552067ac337..c277cdc4728 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -307,7 +307,6 @@ public abstract class FieldMapper extends Mapper { if (ref.get().equals(fieldType()) == false) { throw new IllegalStateException("Cannot overwrite field type reference to unequal reference"); } - ref.incrementAssociatedMappers(); this.fieldTypeRef = ref; } @@ -380,11 +379,6 @@ public abstract class FieldMapper extends Mapper { return; } - boolean strict = this.fieldTypeRef.getNumAssociatedMappers() > 1 && mergeResult.updateAllTypes() == false; - fieldType().checkCompatibility(fieldMergeWith.fieldType(), subConflicts, strict); - for (String conflict : subConflicts) { - mergeResult.addConflict(conflict); - } 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 3fad73ebba6..eaaa47c3bd2 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java @@ -24,6 +24,7 @@ import org.elasticsearch.common.regex.Regex; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -38,18 +39,49 @@ class FieldTypeLookup implements Iterable { /** Full field name to field type */ private final CopyOnWriteHashMap fullNameToFieldType; + /** Full field name to types containing a mapping for this full name. */ + private final CopyOnWriteHashMap> fullNameToTypes; + /** Index field name to field type */ private final CopyOnWriteHashMap indexNameToFieldType; + /** Index field name to types containing a mapping for this index name. */ + private final CopyOnWriteHashMap> indexNameToTypes; + /** Create a new empty instance. */ public FieldTypeLookup() { fullNameToFieldType = new CopyOnWriteHashMap<>(); + fullNameToTypes = new CopyOnWriteHashMap<>(); indexNameToFieldType = new CopyOnWriteHashMap<>(); + indexNameToTypes = new CopyOnWriteHashMap<>(); } - private FieldTypeLookup(CopyOnWriteHashMap fullName, CopyOnWriteHashMap indexName) { - fullNameToFieldType = fullName; - indexNameToFieldType = indexName; + private FieldTypeLookup( + CopyOnWriteHashMap fullName, + CopyOnWriteHashMap> fullNameToTypes, + CopyOnWriteHashMap indexName, + CopyOnWriteHashMap> indexNameToTypes) { + this.fullNameToFieldType = fullName; + this.fullNameToTypes = fullNameToTypes; + this.indexNameToFieldType = indexName; + this.indexNameToTypes = indexNameToTypes; + } + + private static CopyOnWriteHashMap> addType(CopyOnWriteHashMap> map, String key, String type) { + Set types = map.get(key); + if (types == null) { + return map.copyAndPut(key, Collections.singleton(type)); + } else if (types.contains(type)) { + // noting to do + return map; + } else { + Set newTypes = new HashSet<>(types.size() + 1); + newTypes.addAll(types); + newTypes.add(type); + assert newTypes.size() == types.size() + 1; + newTypes = Collections.unmodifiableSet(newTypes); + return map.copyAndPut(key, newTypes); + } } /** @@ -63,7 +95,9 @@ class FieldTypeLookup implements Iterable { throw new IllegalArgumentException("Default mappings should not be added to the lookup"); } CopyOnWriteHashMap fullName = this.fullNameToFieldType; + CopyOnWriteHashMap> fullNameToTypes = this.fullNameToTypes; CopyOnWriteHashMap indexName = this.indexNameToFieldType; + CopyOnWriteHashMap> indexNameToTypes = this.indexNameToTypes; for (FieldMapper fieldMapper : newFieldMappers) { MappedFieldType fieldType = fieldMapper.fieldType(); @@ -91,8 +125,23 @@ class FieldTypeLookup implements Iterable { // this new field bridges between two existing field names (a full and index name), which we cannot support throw new IllegalStateException("insane mappings found. field " + fieldType.names().fullName() + " maps across types to field " + fieldType.names().indexName()); } + + fullNameToTypes = addType(fullNameToTypes, fieldType.names().fullName(), type); + indexNameToTypes = addType(indexNameToTypes, fieldType.names().indexName(), type); + } + return new FieldTypeLookup(fullName, fullNameToTypes, indexName, indexNameToTypes); + } + + private static boolean beStrict(String type, Set types, boolean updateAllTypes) { + assert types.size() >= 1; + if (updateAllTypes) { + return false; + } else if (types.size() == 1 && types.contains(type)) { + // we are implicitly updating all types + return false; + } else { + return true; } - return new FieldTypeLookup(fullName, indexName); } /** @@ -100,14 +149,15 @@ class FieldTypeLookup implements Iterable { * If any are not compatible, an IllegalArgumentException is thrown. * If updateAllTypes is true, only basic compatibility is checked. */ - public void checkCompatibility(Collection newFieldMappers, boolean updateAllTypes) { - for (FieldMapper fieldMapper : newFieldMappers) { + public void checkCompatibility(String type, Collection fieldMappers, boolean updateAllTypes) { + for (FieldMapper fieldMapper : fieldMappers) { 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 - boolean strict = updateAllTypes == false; + 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) { @@ -121,7 +171,8 @@ class FieldTypeLookup implements Iterable { List conflicts = new ArrayList<>(); indexNameRef.get().checkTypeName(fieldMapper.fieldType(), conflicts); if (conflicts.isEmpty()) { // only check compat if they are the same type - boolean strict = updateAllTypes == false; + 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) { @@ -138,6 +189,15 @@ class FieldTypeLookup implements Iterable { return ref.get(); } + /** Get the set of types that have a mapping for the given field. */ + public Set getTypes(String field) { + Set types = fullNameToTypes.get(field); + if (types == null) { + types = Collections.emptySet(); + } + return types; + } + /** Returns the field type for the given index name */ public MappedFieldType getByIndexName(String field) { MappedFieldTypeReference ref = indexNameToFieldType.get(field); @@ -145,6 +205,15 @@ class FieldTypeLookup implements Iterable { return ref.get(); } + /** Get the set of types that have a mapping for the given field. */ + public Set getTypesByIndexName(String field) { + Set types = indexNameToTypes.get(field); + if (types == null) { + types = Collections.emptySet(); + } + return types; + } + /** * Returns a list of the index names of a simple match regex like pattern against full name and index name. */ diff --git a/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldTypeReference.java b/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldTypeReference.java index d3c6b83a6a3..1a9d0b70b37 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldTypeReference.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MappedFieldTypeReference.java @@ -23,12 +23,10 @@ package org.elasticsearch.index.mapper; */ public class MappedFieldTypeReference { private MappedFieldType fieldType; // the current field type this reference points to - private int numAssociatedMappers; public MappedFieldTypeReference(MappedFieldType fieldType) { fieldType.freeze(); // ensure frozen this.fieldType = fieldType; - this.numAssociatedMappers = 1; } public MappedFieldType get() { @@ -40,11 +38,4 @@ public class MappedFieldTypeReference { this.fieldType = fieldType; } - public int getNumAssociatedMappers() { - return numAssociatedMappers; - } - - public void incrementAssociatedMappers() { - ++numAssociatedMappers; - } } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 7a908a1238b..938f610d6db 100755 --- a/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -33,6 +33,7 @@ import org.elasticsearch.ElasticsearchGenerationException; import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.collect.Tuple; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.regex.Regex; @@ -260,13 +261,10 @@ public class MapperService extends AbstractIndexComponent implements Closeable { assert result.hasConflicts() == false; // we already simulated return oldMapper; } else { - List newObjectMappers = new ArrayList<>(); - List newFieldMappers = new ArrayList<>(); - for (MetadataFieldMapper metadataMapper : mapper.mapping().metadataMappers) { - newFieldMappers.add(metadataMapper); - } - MapperUtils.collect(mapper.mapping().root, newObjectMappers, newFieldMappers); - checkNewMappersCompatibility(newObjectMappers, newFieldMappers, updateAllTypes); + Tuple, Collection> newMappers = checkMappersCompatibility( + mapper.type(), mapper.mapping(), updateAllTypes); + Collection newObjectMappers = newMappers.v1(); + Collection newFieldMappers = newMappers.v2(); addMappers(mapper.type(), newObjectMappers, newFieldMappers); for (DocumentTypeListener typeListener : typeListeners) { @@ -302,9 +300,9 @@ public class MapperService extends AbstractIndexComponent implements Closeable { return true; } - protected void checkNewMappersCompatibility(Collection newObjectMappers, Collection newFieldMappers, boolean updateAllTypes) { + protected void checkMappersCompatibility(String type, Collection objectMappers, Collection fieldMappers, boolean updateAllTypes) { assert mappingLock.isWriteLockedByCurrentThread(); - for (ObjectMapper newObjectMapper : newObjectMappers) { + for (ObjectMapper newObjectMapper : objectMappers) { ObjectMapper existingObjectMapper = fullPathObjectMappers.get(newObjectMapper.fullPath()); if (existingObjectMapper != null) { MergeResult result = new MergeResult(true, updateAllTypes); @@ -315,7 +313,19 @@ public class MapperService extends AbstractIndexComponent implements Closeable { } } } - fieldTypes.checkCompatibility(newFieldMappers, updateAllTypes); + fieldTypes.checkCompatibility(type, fieldMappers, updateAllTypes); + } + + protected Tuple, Collection> checkMappersCompatibility( + String type, Mapping mapping, boolean updateAllTypes) { + List objectMappers = new ArrayList<>(); + List fieldMappers = new ArrayList<>(); + for (MetadataFieldMapper metadataMapper : mapping.metadataMappers) { + fieldMappers.add(metadataMapper); + } + MapperUtils.collect(mapping.root, objectMappers, fieldMappers); + checkMappersCompatibility(type, objectMappers, fieldMappers, updateAllTypes); + return new Tuple<>(objectMappers, fieldMappers); } protected void addMappers(String type, Collection objectMappers, Collection fieldMappers) { diff --git a/core/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java index 7045ec2e58b..87a63de99ec 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/core/NumberFieldMapper.java @@ -135,6 +135,15 @@ public abstract class NumberFieldMapper extends FieldMapper implements AllFieldM super(ref); } + @Override + public void checkCompatibility(MappedFieldType other, + List conflicts, boolean strict) { + super.checkCompatibility(other, conflicts, strict); + if (numericPrecisionStep() != other.numericPrecisionStep()) { + conflicts.add("mapper [" + names().fullName() + "] has different [precision_step] values"); + } + } + public abstract NumberFieldType clone(); @Override @@ -251,11 +260,6 @@ public abstract class NumberFieldMapper extends FieldMapper implements AllFieldM return; } NumberFieldMapper nfmMergeWith = (NumberFieldMapper) mergeWith; - if (this.fieldTypeRef.getNumAssociatedMappers() > 1 && mergeResult.updateAllTypes() == false) { - if (fieldType().numericPrecisionStep() != nfmMergeWith.fieldType().numericPrecisionStep()) { - mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] is used by multiple types. Set update_all_types to true to update precision_step across all types."); - } - } if (mergeResult.simulate() == false && mergeResult.hasConflicts() == false) { this.includeInAll = nfmMergeWith.includeInAll; 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 8d6a0800461..5a31618f14e 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java @@ -37,6 +37,8 @@ public class FieldTypeLookupTests extends ESTestCase { FieldTypeLookup lookup = new FieldTypeLookup(); assertNull(lookup.get("foo")); assertNull(lookup.getByIndexName("foo")); + assertEquals(Collections.emptySet(), lookup.getTypes("foo")); + assertEquals(Collections.emptySet(), lookup.getTypesByIndexName("foo")); Collection names = lookup.simpleMatchToFullName("foo"); assertNotNull(names); assertTrue(names.isEmpty()); @@ -70,6 +72,14 @@ public class FieldTypeLookupTests extends ESTestCase { assertNull(lookup.get("bar")); assertEquals(f.fieldType(), lookup2.getByIndexName("bar")); assertNull(lookup.getByIndexName("foo")); + assertEquals(Collections.emptySet(), lookup.getTypes("foo")); + assertEquals(Collections.emptySet(), lookup.getTypesByIndexName("foo")); + assertEquals(Collections.emptySet(), lookup.getTypes("bar")); + assertEquals(Collections.emptySet(), lookup.getTypesByIndexName("bar")); + assertEquals(Collections.singleton("type"), lookup2.getTypes("foo")); + assertEquals(Collections.emptySet(), lookup2.getTypesByIndexName("foo")); + assertEquals(Collections.emptySet(), lookup2.getTypes("bar")); + assertEquals(Collections.singleton("type"), lookup2.getTypesByIndexName("bar")); assertEquals(1, size(lookup2.iterator())); } @@ -144,7 +154,7 @@ public class FieldTypeLookupTests extends ESTestCase { public void testCheckCompatibilityNewField() { FakeFieldMapper f1 = new FakeFieldMapper("foo", "bar"); FieldTypeLookup lookup = new FieldTypeLookup(); - lookup.checkCompatibility(newList(f1), false); + lookup.checkCompatibility("type", newList(f1), false); } public void testCheckCompatibilityMismatchedTypes() { @@ -155,14 +165,14 @@ public class FieldTypeLookupTests extends ESTestCase { MappedFieldType ft2 = FakeFieldMapper.makeOtherFieldType("foo", "foo"); FieldMapper f2 = new FakeFieldMapper("foo", ft2); try { - lookup.checkCompatibility(newList(f2), false); + lookup.checkCompatibility("type2", newList(f2), false); fail("expected type mismatch"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage().contains("cannot be changed from type [faketype] to [otherfaketype]")); } // fails even if updateAllTypes == true try { - lookup.checkCompatibility(newList(f2), true); + lookup.checkCompatibility("type2", newList(f2), true); fail("expected type mismatch"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage().contains("cannot be changed from type [faketype] to [otherfaketype]")); @@ -178,25 +188,27 @@ public class FieldTypeLookupTests extends ESTestCase { ft2.setBoost(2.0f); FieldMapper f2 = new FakeFieldMapper("foo", ft2); try { - lookup.checkCompatibility(newList(f2), false); + // different type + lookup.checkCompatibility("type2", newList(f2), false); fail("expected conflict"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage().contains("to update [boost] across all types")); } - lookup.checkCompatibility(newList(f2), true); // boost is updateable, so ok if forcing + lookup.checkCompatibility("type", newList(f2), false); // boost is updateable, so ok since we are implicitly updating all types + lookup.checkCompatibility("type2", newList(f2), true); // boost is updateable, so ok if forcing // now with a non changeable setting MappedFieldType ft3 = FakeFieldMapper.makeFieldType("foo", "bar"); ft3.setStored(true); FieldMapper f3 = new FakeFieldMapper("foo", ft3); try { - lookup.checkCompatibility(newList(f3), false); + lookup.checkCompatibility("type2", newList(f3), false); fail("expected conflict"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage().contains("has different [store] values")); } // even with updateAllTypes == true, incompatible try { - lookup.checkCompatibility(newList(f3), true); + lookup.checkCompatibility("type2", newList(f3), true); fail("expected conflict"); } catch (IllegalArgumentException e) { assertTrue(e.getMessage().contains("has different [store] values")); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapperTests.java index 9741b82aad6..93fd71599c4 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapperTests.java @@ -25,12 +25,14 @@ import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Priority; +import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.DocumentMapperParser; import org.elasticsearch.index.mapper.MapperParsingException; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.MergeResult; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.search.SearchHitField; @@ -715,28 +717,25 @@ public class GeoPointFieldMapperTests extends ESSingleNodeTestCase { String stage1Mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true) .field("geohash", true).endObject().endObject().endObject().endObject().string(); - DocumentMapperParser parser = createIndex("test", settings).mapperService().documentMapperParser(); - DocumentMapper stage1 = parser.parse(stage1Mapping); + MapperService mapperService = createIndex("test", settings).mapperService(); + DocumentMapper stage1 = mapperService.merge("type", new CompressedXContent(stage1Mapping), true, false); String stage2Mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", false) .field("geohash", false).endObject().endObject().endObject().endObject().string(); - DocumentMapper stage2 = parser.parse(stage2Mapping); - - MergeResult mergeResult = stage1.merge(stage2.mapping(), false, false); - assertThat(mergeResult.hasConflicts(), equalTo(true)); - assertThat(mergeResult.buildConflicts().length, equalTo(3)); - // todo better way of checking conflict? - assertThat("mapper [point] has different [lat_lon]", isIn(new ArrayList<>(Arrays.asList(mergeResult.buildConflicts())))); - assertThat("mapper [point] has different [geohash]", isIn(new ArrayList<>(Arrays.asList(mergeResult.buildConflicts())))); - assertThat("mapper [point] has different [geohash_precision]", isIn(new ArrayList<>(Arrays.asList(mergeResult.buildConflicts())))); + try { + mapperService.merge("type", new CompressedXContent(stage2Mapping), false, false); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("mapper [point] has different [lat_lon]")); + assertThat(e.getMessage(), containsString("mapper [point] has different [geohash]")); + assertThat(e.getMessage(), containsString("mapper [point] has different [geohash_precision]")); + } // correct mapping and ensure no failures stage2Mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("properties").startObject("point").field("type", "geo_point").field("lat_lon", true) .field("geohash", true).endObject().endObject().endObject().endObject().string(); - stage2 = parser.parse(stage2Mapping); - mergeResult = stage1.merge(stage2.mapping(), false, false); - assertThat(Arrays.toString(mergeResult.buildConflicts()), mergeResult.hasConflicts(), equalTo(false)); + mapperService.merge("type", new CompressedXContent(stage2Mapping), false, false); } public void testGeoHashSearch() throws Exception { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java index c00bd3101ae..54e9e96f8ad 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/geo/GeoShapeFieldMapperTests.java @@ -22,12 +22,14 @@ import org.apache.lucene.spatial.prefix.PrefixTreeStrategy; import org.apache.lucene.spatial.prefix.RecursivePrefixTreeStrategy; import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree; import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree; +import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.DocumentMapperParser; import org.elasticsearch.index.mapper.FieldMapper; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.MergeResult; import org.elasticsearch.test.ESSingleNodeTestCase; @@ -35,6 +37,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.isIn; @@ -376,23 +379,21 @@ public class GeoShapeFieldMapperTests extends ESSingleNodeTestCase { .startObject("shape").field("type", "geo_shape").field("tree", "geohash").field("strategy", "recursive") .field("precision", "1m").field("tree_levels", 8).field("distance_error_pct", 0.01).field("orientation", "ccw") .endObject().endObject().endObject().endObject().string(); - DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser(); - DocumentMapper stage1 = parser.parse(stage1Mapping); + MapperService mapperService = createIndex("test").mapperService(); + DocumentMapper stage1 = mapperService.merge("type", new CompressedXContent(stage1Mapping), true, false); String stage2Mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("properties").startObject("shape").field("type", "geo_shape").field("tree", "quadtree") .field("strategy", "term").field("precision", "1km").field("tree_levels", 26).field("distance_error_pct", 26) .field("orientation", "cw").endObject().endObject().endObject().endObject().string(); - DocumentMapper stage2 = parser.parse(stage2Mapping); - - MergeResult mergeResult = stage1.merge(stage2.mapping(), false, false); - // check correct conflicts - assertThat(mergeResult.hasConflicts(), equalTo(true)); - assertThat(mergeResult.buildConflicts().length, equalTo(4)); - ArrayList conflicts = new ArrayList<>(Arrays.asList(mergeResult.buildConflicts())); - assertThat("mapper [shape] has different [strategy]", isIn(conflicts)); - assertThat("mapper [shape] has different [tree]", isIn(conflicts)); - assertThat("mapper [shape] has different [tree_levels]", isIn(conflicts)); - assertThat("mapper [shape] has different [precision]", isIn(conflicts)); + try { + mapperService.merge("type", new CompressedXContent(stage2Mapping), false, false); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("mapper [shape] has different [strategy]")); + assertThat(e.getMessage(), containsString("mapper [shape] has different [tree]")); + assertThat(e.getMessage(), containsString("mapper [shape] has different [tree_levels]")); + assertThat(e.getMessage(), containsString("mapper [shape] has different [precision]")); + } // verify nothing changed FieldMapper fieldMapper = stage1.mappers().getMapper("shape"); @@ -411,11 +412,7 @@ public class GeoShapeFieldMapperTests extends ESSingleNodeTestCase { stage2Mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("properties").startObject("shape").field("type", "geo_shape").field("precision", "1m") .field("tree_levels", 8).field("distance_error_pct", 0.001).field("orientation", "cw").endObject().endObject().endObject().endObject().string(); - stage2 = parser.parse(stage2Mapping); - mergeResult = stage1.merge(stage2.mapping(), false, false); - - // verify mapping changes, and ensure no failures - assertThat(Arrays.toString(mergeResult.buildConflicts()), mergeResult.hasConflicts(), equalTo(false)); + mapperService.merge("type", new CompressedXContent(stage2Mapping), false, false); fieldMapper = stage1.mappers().getMapper("shape"); assertThat(fieldMapper, instanceOf(GeoShapeFieldMapper.class)); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/multifield/merge/JavaMultiFieldMergeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/multifield/merge/JavaMultiFieldMergeTests.java index 07671a2d4b0..30890dcd22a 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/multifield/merge/JavaMultiFieldMergeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/multifield/merge/JavaMultiFieldMergeTests.java @@ -22,9 +22,11 @@ package org.elasticsearch.index.mapper.multifield.merge; import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.IndexableField; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.DocumentMapperParser; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.MergeResult; import org.elasticsearch.index.mapper.ParseContext.Document; import org.elasticsearch.test.ESSingleNodeTestCase; @@ -32,6 +34,7 @@ import org.elasticsearch.test.ESSingleNodeTestCase; import java.util.Arrays; import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; @@ -113,9 +116,9 @@ public class JavaMultiFieldMergeTests extends ESSingleNodeTestCase { public void testUpgradeFromMultiFieldTypeToMultiFields() throws Exception { String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/multifield/merge/test-mapping1.json"); - DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser(); + MapperService mapperService = createIndex("test").mapperService(); - DocumentMapper docMapper = parser.parse(mapping); + DocumentMapper docMapper = mapperService.merge("person", new CompressedXContent(mapping), true, false); assertNotSame(IndexOptions.NONE, docMapper.mappers().getMapper("name").fieldType().indexOptions()); assertThat(docMapper.mappers().getMapper("name.indexed"), nullValue()); @@ -129,12 +132,7 @@ public class JavaMultiFieldMergeTests extends ESSingleNodeTestCase { mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/multifield/merge/upgrade1.json"); - DocumentMapper docMapper2 = parser.parse(mapping); - - MergeResult mergeResult = docMapper.merge(docMapper2.mapping(), true, false); - assertThat(Arrays.toString(mergeResult.buildConflicts()), mergeResult.hasConflicts(), equalTo(false)); - - docMapper.merge(docMapper2.mapping(), false, false); + mapperService.merge("person", new CompressedXContent(mapping), false, false); assertNotSame(IndexOptions.NONE, docMapper.mappers().getMapper("name").fieldType().indexOptions()); @@ -151,12 +149,7 @@ public class JavaMultiFieldMergeTests extends ESSingleNodeTestCase { assertThat(f, notNullValue()); mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/multifield/merge/upgrade2.json"); - DocumentMapper docMapper3 = parser.parse(mapping); - - mergeResult = docMapper.merge(docMapper3.mapping(), true, false); - assertThat(Arrays.toString(mergeResult.buildConflicts()), mergeResult.hasConflicts(), equalTo(false)); - - docMapper.merge(docMapper3.mapping(), false, false); + mapperService.merge("person", new CompressedXContent(mapping), false, false); assertNotSame(IndexOptions.NONE, docMapper.mappers().getMapper("name").fieldType().indexOptions()); @@ -168,24 +161,19 @@ public class JavaMultiFieldMergeTests extends ESSingleNodeTestCase { mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/multifield/merge/upgrade3.json"); - DocumentMapper docMapper4 = parser.parse(mapping); - mergeResult = docMapper.merge(docMapper4.mapping(), true, false); - assertThat(Arrays.toString(mergeResult.buildConflicts()), mergeResult.hasConflicts(), equalTo(true)); - assertThat(mergeResult.buildConflicts()[0], equalTo("mapper [name] has different [index] values")); - assertThat(mergeResult.buildConflicts()[1], equalTo("mapper [name] has different [store] values")); + try { + mapperService.merge("person", new CompressedXContent(mapping), false, false); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("mapper [name] has different [index] values")); + assertThat(e.getMessage(), containsString("mapper [name] has different [store] values")); + } - mergeResult = docMapper.merge(docMapper4.mapping(), false, false); - assertThat(Arrays.toString(mergeResult.buildConflicts()), mergeResult.hasConflicts(), equalTo(true)); - - assertNotSame(IndexOptions.NONE, docMapper.mappers().getMapper("name").fieldType().indexOptions()); - assertThat(mergeResult.buildConflicts()[0], equalTo("mapper [name] has different [index] values")); - assertThat(mergeResult.buildConflicts()[1], equalTo("mapper [name] has different [store] values")); - - // There are conflicts, but the `name.not_indexed3` has been added, b/c that field has no conflicts + // There are conflicts, so the `name.not_indexed3` has not been added assertNotSame(IndexOptions.NONE, docMapper.mappers().getMapper("name").fieldType().indexOptions()); assertThat(docMapper.mappers().getMapper("name.indexed"), notNullValue()); assertThat(docMapper.mappers().getMapper("name.not_indexed"), notNullValue()); assertThat(docMapper.mappers().getMapper("name.not_indexed2"), notNullValue()); - assertThat(docMapper.mappers().getMapper("name.not_indexed3"), notNullValue()); + assertThat(docMapper.mappers().getMapper("name.not_indexed3"), nullValue()); } } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java index 121f100ffa6..9ac039a49fb 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/string/SimpleStringMappingTests.java @@ -25,6 +25,7 @@ import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.IndexableFieldType; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -478,7 +479,7 @@ public class SimpleStringMappingTests extends ESSingleNodeTestCase { .startObject("properties").startObject("field").field("type", "string").endObject().endObject() .endObject().endObject().string(); - DocumentMapper defaultMapper = parser.parse(mapping); + DocumentMapper defaultMapper = indexService.mapperService().merge("type", new CompressedXContent(mapping), true, false); ParsedDocument doc = defaultMapper.parse("test", "type", "1", XContentFactory.jsonBuilder() .startObject() @@ -507,10 +508,12 @@ public class SimpleStringMappingTests extends ESSingleNodeTestCase { updatedMapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("properties").startObject("field").field("type", "string").startObject("norms").field("enabled", true).endObject() .endObject().endObject().endObject().endObject().string(); - mergeResult = defaultMapper.merge(parser.parse(updatedMapping).mapping(), true, false); - assertTrue(mergeResult.hasConflicts()); - assertEquals(1, mergeResult.buildConflicts().length); - assertTrue(mergeResult.buildConflicts()[0].contains("different [omit_norms]")); + try { + defaultMapper.merge(parser.parse(updatedMapping).mapping(), true, false); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("different [omit_norms]")); + } } /** diff --git a/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java b/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java index df9cc10d8c2..53a3bf7bb6e 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java @@ -41,6 +41,7 @@ import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.DocumentMapperParser; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperParsingException; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.MergeResult; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SourceToParse; @@ -557,7 +558,6 @@ public class TimestampMappingTests extends ESSingleNodeTestCase { public void testMergingConflicts() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("_timestamp").field("enabled", true) - .startObject("fielddata").field("format", "doc_values").endObject() .field("store", "yes") .field("index", "analyzed") .field("path", "foo") @@ -565,9 +565,9 @@ public class TimestampMappingTests extends ESSingleNodeTestCase { .endObject() .endObject().endObject().string(); Settings indexSettings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_1_4_2.id).build(); - DocumentMapperParser parser = createIndex("test", indexSettings).mapperService().documentMapperParser(); + MapperService mapperService = createIndex("test", indexSettings).mapperService(); - DocumentMapper docMapper = parser.parse(mapping); + DocumentMapper docMapper = mapperService.merge("type", new CompressedXContent(mapping), true, false); assertThat(docMapper.timestampFieldMapper().fieldType().fieldDataType().getLoading(), equalTo(MappedFieldType.Loading.LAZY)); mapping = XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("_timestamp").field("enabled", false) @@ -579,20 +579,32 @@ public class TimestampMappingTests extends ESSingleNodeTestCase { .endObject() .endObject().endObject().string(); - MergeResult mergeResult = docMapper.merge(parser.parse(mapping).mapping(), true, false); - List expectedConflicts = new ArrayList<>(Arrays.asList( - "mapper [_timestamp] has different [index] values", - "mapper [_timestamp] has different [store] values", - "Cannot update default in _timestamp value. Value is 1970-01-01 now encountering 1970-01-02", - "Cannot update path in _timestamp value. Value is foo path in merged mapping is bar")); - - for (String conflict : mergeResult.buildConflicts()) { - assertTrue("found unexpected conflict [" + conflict + "]", expectedConflicts.remove(conflict)); + try { + mapperService.merge("type", new CompressedXContent(mapping), false, false); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("mapper [_timestamp] has different [index] values")); + assertThat(e.getMessage(), containsString("mapper [_timestamp] has different [store] values")); } - assertTrue("missing conflicts: " + Arrays.toString(expectedConflicts.toArray()), expectedConflicts.isEmpty()); + assertThat(docMapper.timestampFieldMapper().fieldType().fieldDataType().getLoading(), equalTo(MappedFieldType.Loading.LAZY)); assertTrue(docMapper.timestampFieldMapper().enabled()); - assertThat(docMapper.timestampFieldMapper().fieldType().fieldDataType().getFormat(indexSettings), equalTo("doc_values")); + + mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .startObject("_timestamp").field("enabled", true) + .field("store", "yes") + .field("index", "analyzed") + .field("path", "bar") + .field("default", "1970-01-02") + .endObject() + .endObject().endObject().string(); + try { + mapperService.merge("type", new CompressedXContent(mapping), false, false); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("Cannot update default in _timestamp value. Value is 1970-01-01 now encountering 1970-01-02")); + assertThat(e.getMessage(), containsString("Cannot update path in _timestamp value. Value is foo path in merged mapping is bar")); + } } public void testBackcompatMergingConflictsForIndexValues() throws Exception { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterIT.java b/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterIT.java index 1edc2bb131a..35034dfd911 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterIT.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnClusterIT.java @@ -48,7 +48,7 @@ public class UpdateMappingOnClusterIT extends ESIntegTestCase { public void testAllConflicts() throws Exception { String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/update/all_mapping_create_index.json"); String mappingUpdate = copyToStringFromClasspath("/org/elasticsearch/index/mapper/update/all_mapping_update_with_conflicts.json"); - String[] errorMessage = {"[_all] enabled is true now encountering false", + String[] errorMessage = { "[_all] has different [omit_norms] values", "[_all] has different [store] values", "[_all] has different [store_term_vector] values", @@ -61,6 +61,13 @@ public class UpdateMappingOnClusterIT extends ESIntegTestCase { testConflict(mapping, mappingUpdate, errorMessage); } + public void testAllDisabled() throws Exception { + XContentBuilder mapping = jsonBuilder().startObject().startObject("mappings").startObject(TYPE).startObject("_all").field("enabled", true).endObject().endObject().endObject().endObject(); + XContentBuilder mappingUpdate = jsonBuilder().startObject().startObject("_all").field("enabled", false).endObject().startObject("properties").startObject("text").field("type", "string").endObject().endObject().endObject(); + String errorMessage = "[_all] enabled is true now encountering false"; + testConflict(mapping.string(), mappingUpdate.string(), errorMessage); + } + public void testAllWithDefault() throws Exception { String defaultMapping = jsonBuilder().startObject().startObject("_default_") .startObject("_all") 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 b44081fc910..f0a8f5d079d 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 @@ -123,14 +123,14 @@ public class UpdateMappingTests extends ESSingleNodeTestCase { mapperService.merge("type", new CompressedXContent(update.string()), false, false); fail(); } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), containsString("Merge failed")); + assertThat(e.getMessage(), containsString("mapper [foo] cannot be changed from type [long] to [double]")); } try { mapperService.merge("type", new CompressedXContent(update.string()), false, false); fail(); } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), containsString("Merge failed")); + assertThat(e.getMessage(), containsString("mapper [foo] cannot be changed from type [long] to [double]")); } assertTrue(mapperService.documentMapper("type").mapping().root().getMapper("foo") instanceof LongFieldMapper); @@ -167,7 +167,6 @@ public class UpdateMappingTests extends ESSingleNodeTestCase { } // same as the testConflictNewType except that the mapping update is on an existing type - @AwaitsFix(bugUrl="https://github.com/elastic/elasticsearch/issues/15049") public void testConflictNewTypeUpdate() throws Exception { XContentBuilder mapping1 = XContentFactory.jsonBuilder().startObject().startObject("type1") .startObject("properties").startObject("foo").field("type", "long").endObject() diff --git a/core/src/test/java/org/elasticsearch/indices/mapping/UpdateMappingIntegrationIT.java b/core/src/test/java/org/elasticsearch/indices/mapping/UpdateMappingIntegrationIT.java index 57ba469a357..ed4b95c03d8 100644 --- a/core/src/test/java/org/elasticsearch/indices/mapping/UpdateMappingIntegrationIT.java +++ b/core/src/test/java/org/elasticsearch/indices/mapping/UpdateMappingIntegrationIT.java @@ -140,7 +140,7 @@ public class UpdateMappingIntegrationIT extends ESIntegTestCase { .setSource("{\"type\":{\"properties\":{\"body\":{\"type\":\"integer\"}}}}").execute().actionGet(); fail("Expected MergeMappingException"); } catch (IllegalArgumentException e) { - assertThat(e.getMessage(), containsString("mapper [body] of different type")); + assertThat(e.getMessage(), containsString("mapper [body] cannot be changed from type [string] to [int]")); } }