Mappings: Move merge simulation of fieldtype settings to fieldtype method.

For #8871, we need to be able to check field types are compatible,
without comparing FieldMappers.  This change moves the simulation
checks (which generate merge conflicts) for any properties of
MappedFieldTypes into a new method, validateCompatible.

This also simplifies the merge code which merges settings
between the old and new fieldtypes. Previously, each subclass
of FieldMapper would have to set its own fieldtype settings.
However, now that we have .clone(), which perfectly copies
all properties (with subclasses accounted for), we can now
do a simple clone when merging.

Finally, this fixes a subtle bug in merging, in which if
merging has conflicts, and we were not simulating, we would
still update the field type, even though it was not compatible!

NOTE: there is one test failure I am trying to track down with
timestamp merging. Otherwise, all tests pass.
This commit is contained in:
Ryan Ernst 2015-06-19 13:02:56 -07:00
parent 11330d1a34
commit 434b1c94b3
10 changed files with 148 additions and 202 deletions

View File

@ -22,6 +22,7 @@ package org.elasticsearch.index.mapper;
import com.google.common.base.Strings;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.Term;
import org.apache.lucene.index.Terms;
import org.apache.lucene.queries.TermsQuery;
@ -225,6 +226,68 @@ public class MappedFieldType extends FieldType {
// norelease: we need to override freeze() and add safety checks that all settings are actually set
/**
* Checks for any conflicts between this field type and other.
*/
public void validateCompatible(MappedFieldType other, List<String> conflicts) {
boolean indexed = indexOptions() != IndexOptions.NONE;
boolean mergeWithIndexed = other.indexOptions() != IndexOptions.NONE;
if (indexed != mergeWithIndexed || tokenized() != other.tokenized()) {
conflicts.add("mapper [" + names().fullName() + "] has different index values");
}
if (stored() != other.stored()) {
conflicts.add("mapper [" + names().fullName() + "] has different store values");
}
if (hasDocValues() == false && other.hasDocValues()) {
// don't add conflict if this mapper has doc values while the mapper to merge doesn't since doc values are implicitely set
// when the doc_values field data format is configured
conflicts.add("mapper [" + names().fullName() + "] has different doc_values values");
}
if (omitNorms() && !other.omitNorms()) {
conflicts.add("mapper [" + names().fullName() + "] cannot enable norms (`norms.enabled`)");
}
if (tokenized() != other.tokenized()) {
conflicts.add("mapper [" + names().fullName() + "] has different tokenize values");
}
if (storeTermVectors() != other.storeTermVectors()) {
conflicts.add("mapper [" + names().fullName() + "] has different store_term_vector values");
}
if (storeTermVectorOffsets() != other.storeTermVectorOffsets()) {
conflicts.add("mapper [" + names().fullName() + "] has different store_term_vector_offsets values");
}
if (storeTermVectorPositions() != other.storeTermVectorPositions()) {
conflicts.add("mapper [" + names().fullName() + "] has different store_term_vector_positions values");
}
if (storeTermVectorPayloads() != other.storeTermVectorPayloads()) {
conflicts.add("mapper [" + names().fullName() + "] has different store_term_vector_payloads values");
}
// null and "default"-named index analyzers both mean the default is used
if (indexAnalyzer() == null || "default".equals(indexAnalyzer().name())) {
if (other.indexAnalyzer() != null && "default".equals(other.indexAnalyzer().name()) == false) {
conflicts.add("mapper [" + names().fullName() + "] has different analyzer");
}
} else if (other.indexAnalyzer() == null || "default".equals(other.indexAnalyzer().name())) {
conflicts.add("mapper [" + names().fullName() + "] has different analyzer");
} else if (indexAnalyzer().name().equals(other.indexAnalyzer().name()) == false) {
conflicts.add("mapper [" + names().fullName() + "] has different analyzer");
}
if (!names().equals(other.names())) {
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())) {
conflicts.add("mapper [" + names().fullName() + "] has different similarity");
}
}
public boolean isNumeric() {
return false;
}

View File

@ -45,6 +45,7 @@ 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;
@ -392,82 +393,16 @@ public abstract class AbstractFieldMapper implements FieldMapper {
return;
}
AbstractFieldMapper fieldMergeWith = (AbstractFieldMapper) mergeWith;
boolean indexed = fieldType().indexOptions() != IndexOptions.NONE;
boolean mergeWithIndexed = fieldMergeWith.fieldType().indexOptions() != IndexOptions.NONE;
if (indexed != mergeWithIndexed || this.fieldType().tokenized() != fieldMergeWith.fieldType().tokenized()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different index values");
}
if (this.fieldType().stored() != fieldMergeWith.fieldType().stored()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different store values");
}
if (!this.fieldType().hasDocValues() && fieldMergeWith.fieldType().hasDocValues()) {
// don't add conflict if this mapper has doc values while the mapper to merge doesn't since doc values are implicitely set
// when the doc_values field data format is configured
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different " + TypeParsers.DOC_VALUES + " values");
}
if (this.fieldType().omitNorms() && !fieldMergeWith.fieldType().omitNorms()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] cannot enable norms (`norms.enabled`)");
}
if (this.fieldType().tokenized() != fieldMergeWith.fieldType().tokenized()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different tokenize values");
}
if (this.fieldType().storeTermVectors() != fieldMergeWith.fieldType().storeTermVectors()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different store_term_vector values");
}
if (this.fieldType().storeTermVectorOffsets() != fieldMergeWith.fieldType().storeTermVectorOffsets()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different store_term_vector_offsets values");
}
if (this.fieldType().storeTermVectorPositions() != fieldMergeWith.fieldType().storeTermVectorPositions()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different store_term_vector_positions values");
}
if (this.fieldType().storeTermVectorPayloads() != fieldMergeWith.fieldType().storeTermVectorPayloads()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different store_term_vector_payloads values");
}
// null and "default"-named index analyzers both mean the default is used
if (this.fieldType().indexAnalyzer() == null || "default".equals(this.fieldType().indexAnalyzer().name())) {
if (fieldMergeWith.fieldType().indexAnalyzer() != null && "default".equals(fieldMergeWith.fieldType().indexAnalyzer().name()) == false) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different analyzer");
}
} else if (fieldMergeWith.fieldType().indexAnalyzer() == null || "default".equals(fieldMergeWith.fieldType().indexAnalyzer().name())) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different analyzer");
} else if (this.fieldType().indexAnalyzer().name().equals(fieldMergeWith.fieldType().indexAnalyzer().name()) == false) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different analyzer");
}
if (!this.fieldType().names().equals(fieldMergeWith.fieldType().names())) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different index_name");
}
if (this.fieldType().similarity() == null) {
if (fieldMergeWith.fieldType().similarity() != null) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different similarity");
}
} else if (fieldMergeWith.fieldType().similarity() == null) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different similarity");
} else if (!this.fieldType().similarity().equals(fieldMergeWith.fieldType().similarity())) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different similarity");
List<String> subConflicts = new ArrayList<>(); // TODO: just expose list from MergeResult?
fieldType().validateCompatible(fieldMergeWith.fieldType(), subConflicts);
for (String conflict : subConflicts) {
mergeResult.addConflict(conflict);
}
multiFields.merge(mergeWith, mergeResult);
if (!mergeResult.simulate()) {
if (mergeResult.simulate() == false && mergeResult.hasConflicts() == false) {
// apply changeable values
this.fieldType = this.fieldType().clone();
this.fieldType().setOmitNorms(fieldMergeWith.fieldType().omitNorms());
this.fieldType().setBoost(fieldMergeWith.fieldType().boost());
this.fieldType().setNormsLoading(fieldMergeWith.fieldType().normsLoading());
if (fieldMergeWith.fieldType().searchAnalyzer() != null) {
this.fieldType().setSearchAnalyzer(fieldMergeWith.fieldType().searchAnalyzer());
}
if (fieldMergeWith.customFieldDataSettings != null) {
if (!Objects.equal(fieldMergeWith.customFieldDataSettings, this.customFieldDataSettings)) {
this.customFieldDataSettings = fieldMergeWith.customFieldDataSettings;
this.fieldType().setFieldDataType(new FieldDataType(defaultFieldDataType().getType(),
Settings.builder().put(defaultFieldDataType().getSettings()).put(this.customFieldDataSettings)
));
}
}
this.fieldType().setNullValue(fieldMergeWith.fieldType().nullValue());
this.fieldType = fieldMergeWith.fieldType().clone();
this.fieldType().freeze();
this.copyTo = fieldMergeWith.copyTo;
}

View File

@ -242,6 +242,24 @@ public class CompletionFieldMapper extends AbstractFieldMapper {
return new CompletionFieldType(this);
}
@Override
public void validateCompatible(MappedFieldType fieldType, List<String> conflicts) {
super.validateCompatible(fieldType, conflicts);
CompletionFieldType other = (CompletionFieldType)fieldType;
if (analyzingSuggestLookupProvider.hasPayloads() != other.analyzingSuggestLookupProvider.hasPayloads()) {
conflicts.add("mapper [" + names().fullName() + "] has different payload values");
}
if (analyzingSuggestLookupProvider.getPreservePositionsIncrements() != other.analyzingSuggestLookupProvider.getPreservePositionsIncrements()) {
conflicts.add("mapper [" + names().fullName() + "] has different 'preserve_position_increments' values");
}
if (analyzingSuggestLookupProvider.getPreserveSep() != other.analyzingSuggestLookupProvider.getPreserveSep()) {
conflicts.add("mapper [" + names().fullName() + "] has different 'preserve_separators' values");
}
if(!ContextMapping.mappingsAreEqual(getContextMapping(), other.getContextMapping())) {
conflicts.add("mapper [" + names().fullName() + "] has different 'context_mapping' values");
}
}
public void setProvider(AnalyzingCompletionLookupProvider provider) {
checkIfFrozen();
this.analyzingSuggestLookupProvider = provider;
@ -535,18 +553,6 @@ public class CompletionFieldMapper extends AbstractFieldMapper {
public void merge(Mapper mergeWith, MergeResult mergeResult) throws MergeMappingException {
super.merge(mergeWith, mergeResult);
CompletionFieldMapper fieldMergeWith = (CompletionFieldMapper) mergeWith;
if (fieldType().analyzingSuggestLookupProvider.hasPayloads() != fieldMergeWith.fieldType().analyzingSuggestLookupProvider.hasPayloads()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different payload values");
}
if (fieldType().analyzingSuggestLookupProvider.getPreservePositionsIncrements() != fieldMergeWith.fieldType().analyzingSuggestLookupProvider.getPreservePositionsIncrements()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different 'preserve_position_increments' values");
}
if (fieldType().analyzingSuggestLookupProvider.getPreserveSep() != fieldMergeWith.fieldType().analyzingSuggestLookupProvider.getPreserveSep()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different 'preserve_separators' values");
}
if(!ContextMapping.mappingsAreEqual(fieldType().getContextMapping(), fieldMergeWith.fieldType().getContextMapping())) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different 'context_mapping' values");
}
if (!mergeResult.simulate()) {
this.maxInputLength = fieldMergeWith.maxInputLength;
}

View File

@ -491,19 +491,6 @@ public class DateFieldMapper extends NumberFieldMapper {
return CONTENT_TYPE;
}
@Override
public void merge(Mapper mergeWith, MergeResult mergeResult) throws MergeMappingException {
super.merge(mergeWith, mergeResult);
if (!this.getClass().equals(mergeWith.getClass())) {
return;
}
if (!mergeResult.simulate()) {
this.fieldType = this.fieldType.clone();
fieldType().setDateTimeFormatter(((DateFieldMapper) mergeWith).fieldType().dateTimeFormatter());
this.fieldType.freeze();
}
}
@Override
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
super.doXContentBody(builder, includeDefaults, params);

View File

@ -319,9 +319,6 @@ public abstract class NumberFieldMapper extends AbstractFieldMapper implements A
}
if (!mergeResult.simulate()) {
NumberFieldMapper nfmMergeWith = (NumberFieldMapper) mergeWith;
this.fieldType = this.fieldType().clone();
this.fieldType().setNumericPrecisionStep(nfmMergeWith.fieldType().numericPrecisionStep());
this.fieldType().freeze();
this.includeInAll = nfmMergeWith.includeInAll;
if (nfmMergeWith.ignoreMalformed.explicit()) {
this.ignoreMalformed = nfmMergeWith.ignoreMalformed;

View File

@ -330,6 +330,40 @@ public class GeoPointFieldMapper extends AbstractFieldMapper implements ArrayVal
public int hashCode() {
return java.util.Objects.hash(super.hashCode(), geohashFieldType, geohashPrecision, geohashPrefixEnabled, latFieldType, lonFieldType, validateLon, validateLat, normalizeLon, normalizeLat);
}
@Override
public void validateCompatible(MappedFieldType fieldType, List<String> conflicts) {
super.validateCompatible(fieldType, conflicts);
GeoPointFieldType other = (GeoPointFieldType)fieldType;
if (isLatLonEnabled() != other.isLatLonEnabled()) {
conflicts.add("mapper [" + names().fullName() + "] has different lat_lon");
}
if (isGeohashEnabled() != other.isGeohashEnabled()) {
conflicts.add("mapper [" + names().fullName() + "] has different geohash");
}
if (geohashPrecision() != other.geohashPrecision()) {
conflicts.add("mapper [" + names().fullName() + "] has different geohash_precision");
}
if (isGeohashPrefixEnabled() != other.isGeohashPrefixEnabled()) {
conflicts.add("mapper [" + names().fullName() + "] has different geohash_prefix");
}
if (normalizeLat() != other.normalizeLat()) {
conflicts.add("mapper [" + names().fullName() + "] has different normalize_lat");
}
if (normalizeLon() != other.normalizeLon()) {
conflicts.add("mapper [" + names().fullName() + "] has different normalize_lon");
}
if (isLatLonEnabled() &&
latFieldType().numericPrecisionStep() != other.latFieldType().numericPrecisionStep()) {
conflicts.add("mapper [" + names().fullName() + "] has different precision_step");
}
if (validateLat() != other.validateLat()) {
conflicts.add("mapper [" + names().fullName() + "] has different validate_lat");
}
if (validateLon() != other.validateLon()) {
conflicts.add("mapper [" + names().fullName() + "] has different validate_lon");
}
}
public boolean isGeohashEnabled() {
return geohashFieldType != null;
@ -718,44 +752,6 @@ public class GeoPointFieldMapper extends AbstractFieldMapper implements ArrayVal
}
}
@Override
public void merge(Mapper mergeWith, MergeResult mergeResult) throws MergeMappingException {
super.merge(mergeWith, mergeResult);
if (!this.getClass().equals(mergeWith.getClass())) {
return;
}
GeoPointFieldMapper fieldMergeWith = (GeoPointFieldMapper) mergeWith;
if (this.fieldType().isLatLonEnabled() != fieldMergeWith.fieldType().isLatLonEnabled()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different lat_lon");
}
if (this.fieldType().isGeohashEnabled() != fieldMergeWith.fieldType().isGeohashEnabled()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different geohash");
}
if (this.fieldType().geohashPrecision() != fieldMergeWith.fieldType().geohashPrecision()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different geohash_precision");
}
if (this.fieldType().isGeohashPrefixEnabled() != fieldMergeWith.fieldType().isGeohashPrefixEnabled()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different geohash_prefix");
}
if (this.fieldType().normalizeLat() != fieldMergeWith.fieldType().normalizeLat()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different normalize_lat");
}
if (this.fieldType().normalizeLon() != fieldMergeWith.fieldType().normalizeLon()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different normalize_lon");
}
if (fieldType().isLatLonEnabled() &&
this.fieldType().latFieldType().numericPrecisionStep() != fieldMergeWith.fieldType().latFieldType().numericPrecisionStep()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different precision_step");
}
if (this.fieldType().validateLat() != fieldMergeWith.fieldType().validateLat()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different validate_lat");
}
if (this.fieldType().validateLon() != fieldMergeWith.fieldType().validateLon()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different validate_lon");
}
}
@Override
public Iterator<Mapper> iterator() {
List<Mapper> extras = new ArrayList<>();

View File

@ -246,6 +246,30 @@ public class GeoShapeFieldMapper extends AbstractFieldMapper {
termStrategy.setDistErrPct(distanceErrorPct());
defaultStrategy = resolveStrategy(strategyName);
}
@Override
public void validateCompatible(MappedFieldType fieldType, List<String> conflicts) {
super.validateCompatible(fieldType, conflicts);
GeoShapeFieldType other = (GeoShapeFieldType)fieldType;
// prevent user from changing strategies
if (strategyName().equals(other.strategyName()) == false) {
conflicts.add("mapper [" + names().fullName() + "] has different strategy");
}
// prevent user from changing trees (changes encoding)
if (tree().equals(other.tree()) == false) {
conflicts.add("mapper [" + names().fullName() + "] has different tree");
}
// TODO we should allow this, but at the moment levels is used to build bookkeeping variables
// in lucene's SpatialPrefixTree implementations, need a patch to correct that first
if (treeLevels() != other.treeLevels()) {
conflicts.add("mapper [" + names().fullName() + "] has different tree_levels");
}
if (precisionInMeters() != other.precisionInMeters()) {
conflicts.add("mapper [" + names().fullName() + "] has different precision");
}
}
private static int getLevels(int treeLevels, double precisionInMeters, int defaultLevels, boolean geoHash) {
if (treeLevels > 0 || precisionInMeters >= 0) {
@ -379,48 +403,6 @@ public class GeoShapeFieldMapper extends AbstractFieldMapper {
return null;
}
@Override
public void merge(Mapper mergeWith, MergeResult mergeResult) throws MergeMappingException {
super.merge(mergeWith, mergeResult);
if (!this.getClass().equals(mergeWith.getClass())) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different field type");
return;
}
final GeoShapeFieldMapper fieldMergeWith = (GeoShapeFieldMapper) mergeWith;
// prevent user from changing strategies
if (fieldType().strategyName().equals(fieldMergeWith.fieldType().strategyName()) == false) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different strategy");
}
// prevent user from changing trees (changes encoding)
if (fieldType().tree().equals(fieldMergeWith.fieldType().tree()) == false) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different tree");
}
// TODO we should allow this, but at the moment levels is used to build bookkeeping variables
// in lucene's SpatialPrefixTree implementations, need a patch to correct that first
if (fieldType().treeLevels() != fieldMergeWith.fieldType().treeLevels()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different tree_levels");
}
if (fieldType().precisionInMeters() != fieldMergeWith.fieldType().precisionInMeters()) {
mergeResult.addConflict("mapper [" + fieldType().names().fullName() + "] has different precision");
}
// bail if there were merge conflicts
if (mergeResult.hasConflicts() || mergeResult.simulate()) {
return;
}
// change distance error percent
this.fieldType = fieldType().clone();
this.fieldType().setDistanceErrorPct(fieldMergeWith.fieldType().distanceErrorPct());
// change orientation - this is allowed because existing dateline spanning shapes
// have already been unwound and segmented
this.fieldType().setOrientation(fieldMergeWith.fieldType().orientation());
fieldType().freeze();
}
@Override
protected void parseCreateField(ParseContext context, List<Field> fields) throws IOException {
}

View File

@ -315,18 +315,6 @@ public class FieldNamesFieldMapper extends AbstractFieldMapper implements RootMa
return builder;
}
@Override
public void merge(Mapper mergeWith, MergeResult mergeResult) throws MergeMappingException {
FieldNamesFieldMapper fieldNamesMapperMergeWith = (FieldNamesFieldMapper)mergeWith;
if (!mergeResult.simulate()) {
if (fieldNamesMapperMergeWith.fieldType().isEnabled() != fieldType().isEnabled()) {
this.fieldType = fieldType().clone();
fieldType().setEnabled(fieldNamesMapperMergeWith.fieldType().isEnabled());
fieldType().freeze();
}
}
}
@Override
public boolean isGenerated() {
return true;

View File

@ -346,15 +346,7 @@ public class ParentFieldMapper extends AbstractFieldMapper implements RootMapper
if (!mergeResult.simulate()) {
ParentFieldMapper fieldMergeWith = (ParentFieldMapper) mergeWith;
this.fieldType = this.fieldType().clone();
if (fieldMergeWith.customFieldDataSettings != null) {
if (!Objects.equal(fieldMergeWith.customFieldDataSettings, this.customFieldDataSettings)) {
this.customFieldDataSettings = fieldMergeWith.customFieldDataSettings;
this.fieldType().setFieldDataType(new FieldDataType(defaultFieldDataType().getType(),
builder().put(defaultFieldDataType().getSettings()).put(this.customFieldDataSettings)
));
}
}
this.fieldType = fieldMergeWith.fieldType().clone();
this.fieldType().freeze();
}
}

View File

@ -499,6 +499,6 @@ public class GeoPointFieldMapperTests extends ElasticsearchSingleNodeTest {
.endObject().endObject().string();
stage2 = parser.parse(stage2Mapping);
mergeResult = stage1.merge(stage2.mapping(), false);
assertThat(mergeResult.hasConflicts(), equalTo(false));
assertThat(Arrays.toString(mergeResult.buildConflicts()), mergeResult.hasConflicts(), equalTo(false));
}
}