Mapping update for “date_range” field type is not idempotent (#2094)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
This commit is contained in:
Andriy Redko 2022-02-14 17:56:02 -05:00 committed by GitHub
parent a942b275ef
commit 6b6f03368f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 54 additions and 12 deletions

View File

@ -195,7 +195,12 @@ public class PercolatorFieldMapper extends ParametrizedFieldMapper {
}
static RangeFieldMapper createExtractedRangeFieldBuilder(String name, RangeType rangeType, BuilderContext context) {
RangeFieldMapper.Builder builder = new RangeFieldMapper.Builder(name, rangeType, true);
RangeFieldMapper.Builder builder = new RangeFieldMapper.Builder(
name,
rangeType,
true,
hasIndexCreated(context.indexSettings()) ? context.indexCreatedVersion() : null
);
// For now no doc values, because in processQuery(...) only the Lucene range fields get added:
builder.docValues(false);
return builder.build(context);

View File

@ -33,6 +33,8 @@
package org.opensearch.index.mapper;
import org.opensearch.Version;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.common.Nullable;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.time.DateFormatter;
import org.opensearch.common.xcontent.ToXContentFragment;
@ -69,6 +71,14 @@ public abstract class Mapper implements ToXContentFragment, Iterable<Mapper> {
public Version indexCreatedVersion() {
return Version.indexCreated(indexSettings);
}
public Version indexCreatedVersionOrDefault(@Nullable Version defaultValue) {
if (defaultValue == null || hasIndexCreated(indexSettings)) {
return indexCreatedVersion();
} else {
return defaultValue;
}
}
}
public abstract static class Builder<T extends Builder> {
@ -240,4 +250,12 @@ public abstract class Mapper implements ToXContentFragment, Iterable<Mapper> {
*/
public abstract void validate(MappingLookup mappers);
/**
* Check if settings have IndexMetadata.SETTING_INDEX_VERSION_CREATED setting.
* @param settings settings
* @return "true" if settings have IndexMetadata.SETTING_INDEX_VERSION_CREATED setting, "false" otherwise
*/
protected static boolean hasIndexCreated(Settings settings) {
return settings.hasValue(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey());
}
}

View File

@ -36,6 +36,7 @@ import org.apache.lucene.search.BoostQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.util.BytesRef;
import org.opensearch.OpenSearchException;
import org.opensearch.Version;
import org.opensearch.common.Explicit;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.geo.ShapeRelation;
@ -116,15 +117,17 @@ public class RangeFieldMapper extends ParametrizedFieldMapper {
private final Parameter<Map<String, String>> meta = Parameter.metaParam();
private final RangeType type;
private final Version indexCreatedVersion;
public Builder(String name, RangeType type, Settings settings) {
this(name, type, COERCE_SETTING.get(settings));
this(name, type, COERCE_SETTING.get(settings), hasIndexCreated(settings) ? Version.indexCreated(settings) : null);
}
public Builder(String name, RangeType type, boolean coerceByDefault) {
public Builder(String name, RangeType type, boolean coerceByDefault, Version indexCreatedVersion) {
super(name);
this.type = type;
this.coerce = Parameter.explicitBoolParam("coerce", true, m -> toType(m).coerce, coerceByDefault);
this.indexCreatedVersion = indexCreatedVersion;
if (this.type != RangeType.DATE) {
format.neverSerialize();
locale.neverSerialize();
@ -157,8 +160,11 @@ public class RangeFieldMapper extends ParametrizedFieldMapper {
+ " type"
);
}
// The builder context may not have index created version, falling back to indexCreatedVersion
// property of this mapper builder.
DateFormatter dateTimeFormatter;
if (Joda.isJodaPattern(context.indexCreatedVersion(), format.getValue())) {
if (Joda.isJodaPattern(context.indexCreatedVersionOrDefault(indexCreatedVersion), format.getValue())) {
dateTimeFormatter = Joda.forPattern(format.getValue()).withLocale(locale.getValue());
} else {
dateTimeFormatter = DateFormatter.forPattern(format.getValue()).withLocale(locale.getValue());
@ -371,6 +377,7 @@ public class RangeFieldMapper extends ParametrizedFieldMapper {
private final Locale locale;
private final boolean coerceByDefault;
private final Version indexCreatedVersion;
private RangeFieldMapper(
String simpleName,
@ -389,6 +396,7 @@ public class RangeFieldMapper extends ParametrizedFieldMapper {
this.format = builder.format.getValue();
this.locale = builder.locale.getValue();
this.coerceByDefault = builder.coerce.getDefaultValue().value();
this.indexCreatedVersion = builder.indexCreatedVersion;
}
boolean coerce() {
@ -397,7 +405,7 @@ public class RangeFieldMapper extends ParametrizedFieldMapper {
@Override
public ParametrizedFieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), type, coerceByDefault).init(this);
return new Builder(simpleName(), type, coerceByDefault, indexCreatedVersion).init(this);
}
@Override

View File

@ -46,7 +46,7 @@ public class IpRangeFieldTypeTests extends FieldTypeTestCase {
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
RangeFieldMapper mapper = new RangeFieldMapper.Builder("field", RangeType.IP, true).build(context);
RangeFieldMapper mapper = new RangeFieldMapper.Builder("field", RangeType.IP, true, Version.V_EMPTY).build(context);
Map<String, Object> range = org.opensearch.common.collect.Map.of("gte", "2001:db8:0:0:0:0:2:1");
assertEquals(
Collections.singletonList(org.opensearch.common.collect.Map.of("gte", "2001:db8::2:1")),

View File

@ -41,6 +41,7 @@ import org.opensearch.common.network.InetAddresses;
import org.opensearch.common.xcontent.ToXContent;
import org.opensearch.common.xcontent.XContentBuilder;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.index.mapper.MapperService.MergeReason;
import java.io.IOException;
import java.net.InetAddress;
@ -374,4 +375,12 @@ public class RangeFieldMapperTests extends AbstractNumericFieldMapperTestCase {
assertThat(e.getMessage(), containsString("Invalid format: [[test_format]]: Unknown pattern letter: t"));
}
public void testUpdatesWithSameMappings() throws Exception {
for (final String type : types()) {
final DocumentMapper mapper = createDocumentMapper(rangeFieldMapping(type, b -> { b.field("store", true); }));
final Mapping mapping = mapper.mapping();
mapper.merge(mapping, MergeReason.MAPPING_UPDATE);
}
}
}

View File

@ -536,16 +536,17 @@ public class RangeFieldTypeTests extends FieldTypeTestCase {
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
MappedFieldType longMapper = new RangeFieldMapper.Builder("field", RangeType.LONG, true).build(context).fieldType();
MappedFieldType longMapper = new RangeFieldMapper.Builder("field", RangeType.LONG, true, Version.V_EMPTY).build(context)
.fieldType();
Map<String, Object> longRange = org.opensearch.common.collect.Map.of("gte", 3.14, "lt", "42.9");
assertEquals(
Collections.singletonList(org.opensearch.common.collect.Map.of("gte", 3L, "lt", 42L)),
fetchSourceValue(longMapper, longRange)
);
MappedFieldType dateMapper = new RangeFieldMapper.Builder("field", RangeType.DATE, true).format("yyyy/MM/dd||epoch_millis")
.build(context)
.fieldType();
MappedFieldType dateMapper = new RangeFieldMapper.Builder("field", RangeType.DATE, true, Version.V_EMPTY).format(
"yyyy/MM/dd||epoch_millis"
).build(context).fieldType();
Map<String, Object> dateRange = org.opensearch.common.collect.Map.of("lt", "1990/12/29", "gte", 597429487111L);
assertEquals(
Collections.singletonList(org.opensearch.common.collect.Map.of("lt", "1990/12/29", "gte", "1988/12/06")),
@ -557,14 +558,15 @@ public class RangeFieldTypeTests extends FieldTypeTestCase {
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
MappedFieldType longMapper = new RangeFieldMapper.Builder("field", RangeType.LONG, true).build(context).fieldType();
MappedFieldType longMapper = new RangeFieldMapper.Builder("field", RangeType.LONG, true, Version.V_EMPTY).build(context)
.fieldType();
Map<String, Object> longRange = org.opensearch.common.collect.Map.of("gte", 3.14, "lt", "42.9");
assertEquals(
Collections.singletonList(org.opensearch.common.collect.Map.of("gte", 3L, "lt", 42L)),
fetchSourceValue(longMapper, longRange)
);
MappedFieldType dateMapper = new RangeFieldMapper.Builder("field", RangeType.DATE, true).format("strict_date_time")
MappedFieldType dateMapper = new RangeFieldMapper.Builder("field", RangeType.DATE, true, Version.V_EMPTY).format("strict_date_time")
.build(context)
.fieldType();
Map<String, Object> dateRange = org.opensearch.common.collect.Map.of("lt", "1990-12-29T00:00:00.000Z");