Addressed PR comments

This commit is contained in:
Ryan Ernst 2015-06-23 08:59:30 -07:00
parent 18ec76aae8
commit 33339ab288
12 changed files with 30 additions and 34 deletions

View File

@ -129,6 +129,7 @@ public class CreateIndexClusterStateUpdateRequest extends ClusterStateUpdateRequ
return blocks;
}
/** True if all fields that span multiple types should be updated, false otherwise */
public boolean updateAllTypes() {
return updateAllTypes;
}

View File

@ -529,7 +529,7 @@ class DocumentParser implements Closeable {
builder = MapperBuilders.longField(currentFieldName);
}
return builder;
} catch (Exception e) {
} catch (NumberFormatException e) {
// not a long number
}
try {
@ -539,7 +539,7 @@ class DocumentParser implements Closeable {
builder = MapperBuilders.doubleField(currentFieldName);
}
return builder;
} catch (Exception e) {
} catch (NumberFormatException e) {
// not a long number
}
}

View File

@ -24,10 +24,8 @@ import com.google.common.collect.Iterators;
import com.google.common.collect.Sets;
import org.elasticsearch.common.collect.CopyOnWriteHashMap;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.index.mapper.object.ObjectMapper;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
@ -101,7 +99,7 @@ class FieldTypeLookup implements Iterable<MappedFieldType> {
}
/**
* Checks if the give mapper's field types are compatibile with existing field types.
* Checks if the given mappers' field types are compatible with existing field types.
* If any are not compatible, an IllegalArgumentException is thrown.
* If updateAllTypes is true, only basic compatibility is checked.
*/
@ -109,7 +107,7 @@ class FieldTypeLookup implements Iterable<MappedFieldType> {
for (FieldMapper fieldMapper : newFieldMappers) {
MappedFieldTypeReference ref = fullNameToFieldType.get(fieldMapper.fieldType().names().fullName());
if (ref != null) {
boolean strict = ref.getRefCount() > 1 && updateAllTypes == false;
boolean strict = ref.getNumAssociatedMappers() > 1 && updateAllTypes == false;
List<String> conflicts = new ArrayList<>();
ref.get().checkCompatibility(fieldMapper.fieldType(), conflicts, strict);
if (conflicts.isEmpty() == false) {
@ -120,7 +118,7 @@ class FieldTypeLookup implements Iterable<MappedFieldType> {
// field type for the index name must be compatible too
MappedFieldTypeReference indexNameRef = fullNameToFieldType.get(fieldMapper.fieldType().names().indexName());
if (indexNameRef != null) {
boolean strict = indexNameRef.getRefCount() > 1 && updateAllTypes == false;
boolean strict = indexNameRef.getNumAssociatedMappers() > 1 && updateAllTypes == false;
List<String> conflicts = new ArrayList<>();
indexNameRef.get().checkCompatibility(fieldMapper.fieldType(), conflicts, strict);
if (conflicts.isEmpty() == false) {

View File

@ -23,12 +23,12 @@ package org.elasticsearch.index.mapper;
*/
public class MappedFieldTypeReference {
private MappedFieldType fieldType; // the current field type this reference points to
private int refCount;
private int numAssociatedMappers;
public MappedFieldTypeReference(MappedFieldType fieldType) {
fieldType.freeze(); // ensure frozen
this.fieldType = fieldType;
this.refCount = 1;
this.numAssociatedMappers = 1;
}
public MappedFieldType get() {
@ -40,11 +40,11 @@ public class MappedFieldTypeReference {
this.fieldType = fieldType;
}
public int getRefCount() {
return refCount;
public int getNumAssociatedMappers() {
return numAssociatedMappers;
}
public void incRefCount() {
++refCount;
public void incrementAssociatedMappers() {
++numAssociatedMappers;
}
}

View File

@ -350,7 +350,7 @@ public abstract class AbstractFieldMapper implements FieldMapper {
if (ref.get().equals(fieldType()) == false) {
throw new IllegalStateException("Cannot overwrite field type reference to unequal reference");
}
ref.incRefCount();
ref.incrementAssociatedMappers();
this.fieldTypeRef = ref;
}
@ -409,7 +409,7 @@ public abstract class AbstractFieldMapper implements FieldMapper {
}
AbstractFieldMapper fieldMergeWith = (AbstractFieldMapper) mergeWith;
List<String> subConflicts = new ArrayList<>(); // TODO: just expose list from MergeResult?
boolean strict = this.fieldTypeRef.getRefCount() > 1 && mergeResult.updateAllTypes() == false;
boolean strict = this.fieldTypeRef.getNumAssociatedMappers() > 1 && mergeResult.updateAllTypes() == false;
fieldType().checkCompatibility(fieldMergeWith.fieldType(), subConflicts, strict);
for (String conflict : subConflicts) {
mergeResult.addConflict(conflict);
@ -482,8 +482,7 @@ public abstract class AbstractFieldMapper implements FieldMapper {
}
TreeMap<String, Object> orderedFielddataSettings = new TreeMap<>();
boolean hasCustomFieldDataSettings = customFieldDataSettings != null && customFieldDataSettings.equals(Settings.EMPTY) == false;
if (hasCustomFieldDataSettings) {
if (hasCustomFieldDataSettings()) {
orderedFielddataSettings.putAll(customFieldDataSettings.getAsMap());
builder.field("fielddata", orderedFielddataSettings);
} else if (includeDefaults) {
@ -563,6 +562,10 @@ public abstract class AbstractFieldMapper implements FieldMapper {
}
}
protected boolean hasCustomFieldDataSettings() {
return customFieldDataSettings != null && customFieldDataSettings.equals(Settings.EMPTY) == false;
}
protected abstract String contentType();
@Override

View File

@ -319,7 +319,7 @@ public abstract class NumberFieldMapper extends AbstractFieldMapper implements A
return;
}
NumberFieldMapper nfmMergeWith = (NumberFieldMapper) mergeWith;
if (this.fieldTypeRef.getRefCount() > 1 && mergeResult.updateAllTypes() == false) {
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.");
}

View File

@ -314,8 +314,7 @@ public class AllFieldMapper extends AbstractFieldMapper implements RootMapper {
builder.field("similarity", SimilarityLookupService.DEFAULT_SIMILARITY);
}
boolean hasCustomFieldDataSettings = customFieldDataSettings != null && customFieldDataSettings.equals(Settings.EMPTY) == false;
if (hasCustomFieldDataSettings) {
if (hasCustomFieldDataSettings()) {
builder.field("fielddata", (Map) customFieldDataSettings.getAsMap());
} else if (includeDefaults) {
builder.field("fielddata", (Map) fieldType().fieldDataType().getSettings().getAsMap());

View File

@ -310,13 +310,12 @@ public class IdFieldMapper extends AbstractFieldMapper implements RootMapper {
return builder;
}
boolean includeDefaults = params.paramAsBoolean("include_defaults", false);
boolean hasCustomFieldDataSettings = customFieldDataSettings != null && customFieldDataSettings.equals(Settings.EMPTY) == false;
// if all are defaults, no sense to write it at all
if (!includeDefaults && fieldType().stored() == Defaults.FIELD_TYPE.stored()
&& fieldType().indexOptions() == Defaults.FIELD_TYPE.indexOptions()
&& path == Defaults.PATH
&& hasCustomFieldDataSettings == false) {
&& hasCustomFieldDataSettings() == false) {
return builder;
}
builder.startObject(CONTENT_TYPE);
@ -330,7 +329,7 @@ public class IdFieldMapper extends AbstractFieldMapper implements RootMapper {
builder.field("path", path);
}
if (hasCustomFieldDataSettings) {
if (hasCustomFieldDataSettings()) {
builder.field("fielddata", (Map) customFieldDataSettings.getAsMap());
} else if (includeDefaults) {
builder.field("fielddata", (Map) fieldType().fieldDataType().getSettings().getAsMap());

View File

@ -206,10 +206,9 @@ public class IndexFieldMapper extends AbstractFieldMapper implements RootMapper
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
boolean includeDefaults = params.paramAsBoolean("include_defaults", false);
boolean hasCustomFieldDataSettings = customFieldDataSettings != null && customFieldDataSettings.equals(Settings.EMPTY) == false;
// if all defaults, no need to write it at all
if (!includeDefaults && fieldType().stored() == Defaults.FIELD_TYPE.stored() && enabledState == Defaults.ENABLED_STATE && hasCustomFieldDataSettings == false) {
if (!includeDefaults && fieldType().stored() == Defaults.FIELD_TYPE.stored() && enabledState == Defaults.ENABLED_STATE && hasCustomFieldDataSettings() == false) {
return builder;
}
builder.startObject(CONTENT_TYPE);
@ -221,7 +220,7 @@ public class IndexFieldMapper extends AbstractFieldMapper implements RootMapper
}
if (indexCreatedBefore2x) {
if (hasCustomFieldDataSettings) {
if (hasCustomFieldDataSettings()) {
builder.field("fielddata", (Map) customFieldDataSettings.getAsMap());
} else if (includeDefaults) {
builder.field("fielddata", (Map) fieldType().fieldDataType().getSettings().getAsMap());

View File

@ -324,11 +324,10 @@ public class ParentFieldMapper extends AbstractFieldMapper implements RootMapper
return builder;
}
boolean includeDefaults = params.paramAsBoolean("include_defaults", false);
boolean hasCustomFieldDataSettings = customFieldDataSettings != null && customFieldDataSettings.equals(Settings.EMPTY) == false;
builder.startObject(CONTENT_TYPE);
builder.field("type", type);
if (hasCustomFieldDataSettings) {
if (hasCustomFieldDataSettings()) {
builder.field("fielddata", (Map) customFieldDataSettings.getAsMap());
} else if (includeDefaults) {
builder.field("fielddata", (Map) fieldType().fieldDataType().getSettings().getAsMap());

View File

@ -331,10 +331,9 @@ public class TimestampFieldMapper extends DateFieldMapper implements RootMapper
boolean includeDefaults = params.paramAsBoolean("include_defaults", false);
boolean indexed = fieldType().indexOptions() != IndexOptions.NONE;
boolean indexedDefault = Defaults.FIELD_TYPE.indexOptions() != IndexOptions.NONE;
boolean hasCustomFieldDataSettings = customFieldDataSettings != null && customFieldDataSettings.equals(Settings.EMPTY) == false;
// if all are defaults, no sense to write it at all
if (!includeDefaults && indexed == indexedDefault && hasCustomFieldDataSettings == false &&
if (!includeDefaults && indexed == indexedDefault && hasCustomFieldDataSettings() == false &&
fieldType().stored() == Defaults.FIELD_TYPE.stored() && enabledState == Defaults.ENABLED && path == Defaults.PATH
&& fieldType().dateTimeFormatter().format().equals(Defaults.DATE_TIME_FORMATTER.format())
&& Defaults.DEFAULT_TIMESTAMP.equals(defaultTimestamp)
@ -367,7 +366,7 @@ public class TimestampFieldMapper extends DateFieldMapper implements RootMapper
builder.field("ignore_missing", ignoreMissing);
}
if (indexCreatedBefore2x) {
if (hasCustomFieldDataSettings) {
if (hasCustomFieldDataSettings()) {
builder.field("fielddata", (Map) customFieldDataSettings.getAsMap());
} else if (includeDefaults) {
builder.field("fielddata", (Map) fieldType().fieldDataType().getSettings().getAsMap());

View File

@ -219,16 +219,15 @@ public class UidFieldMapper extends AbstractFieldMapper implements RootMapper {
return builder;
}
boolean includeDefaults = params.paramAsBoolean("include_defaults", false);
boolean hasCustomFieldDataSettings = customFieldDataSettings != null && customFieldDataSettings.equals(Settings.EMPTY) == false;
// if defaults, don't output
if (!includeDefaults && hasCustomFieldDataSettings == false) {
if (!includeDefaults && hasCustomFieldDataSettings() == false) {
return builder;
}
builder.startObject(CONTENT_TYPE);
if (hasCustomFieldDataSettings) {
if (hasCustomFieldDataSettings()) {
builder.field("fielddata", (Map) customFieldDataSettings.getAsMap());
} else if (includeDefaults) {
builder.field("fielddata", (Map) fieldType().fieldDataType().getSettings().getAsMap());