diff --git a/docs/changelog/138724.yaml b/docs/changelog/138724.yaml new file mode 100644 index 0000000000000..35bf9a2755b49 --- /dev/null +++ b/docs/changelog/138724.yaml @@ -0,0 +1,5 @@ +pr: 138724 +summary: '`AggregateMetricDouble` fields should not build BKD indexes' +area: Mapping +type: feature +issues: [] diff --git a/server/src/main/java/org/elasticsearch/index/IndexVersions.java b/server/src/main/java/org/elasticsearch/index/IndexVersions.java index 4d4b90aabd029..1fed94f126198 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexVersions.java +++ b/server/src/main/java/org/elasticsearch/index/IndexVersions.java @@ -204,6 +204,7 @@ private static Version parseUnchecked(String version) { public static final IndexVersion DISK_BBQ_LICENSE_ENFORCEMENT = def(9_053_0_00, Version.LUCENE_10_3_2); public static final IndexVersion STORE_IGNORED_KEYWORDS_IN_BINARY_DOC_VALUES = def(9_054_0_00, Version.LUCENE_10_3_2); public static final IndexVersion TIME_SERIES_USE_STORED_FIELDS_BLOOM_FILTER_FOR_ID = def(9_055_0_00, Version.LUCENE_10_3_2); + public static final IndexVersion AGG_METRIC_DOUBLE_REMOVE_POINTS = def(9_056_0_00, Version.LUCENE_10_3_2); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index b80f463204146..d26f427cb63f4 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -214,6 +214,11 @@ public Builder coerce(boolean coerce) { return this; } + public Builder index(boolean index) { + this.indexed.setValue(index); + return this; + } + public Builder docValues(boolean hasDocValues) { this.hasDocValues.setValue(hasDocValues); return this; diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapper.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapper.java index 35de3032fced4..de13cdd9996ff 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapper.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapper.java @@ -20,6 +20,7 @@ import org.elasticsearch.common.time.DateMathParser; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.IndexVersions; import org.elasticsearch.index.fielddata.FieldDataContext; import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.ScriptDocValues; @@ -235,6 +236,9 @@ public AggregateMetricDoubleFieldMapper build(MapperBuilderContext context) { ScriptCompiler.NONE, indexSettings ).allowMultipleValues(false).ignoreMalformed(false).coerce(false); + if (indexSettings.getIndexVersionCreated().onOrAfter(IndexVersions.AGG_METRIC_DOUBLE_REMOVE_POINTS)) { + builder.index(false); + } } else { builder = new NumberFieldMapper.Builder( fieldName, @@ -242,6 +246,9 @@ public AggregateMetricDoubleFieldMapper build(MapperBuilderContext context) { ScriptCompiler.NONE, indexSettings ).allowMultipleValues(false).ignoreMalformed(false).coerce(true); + if (indexSettings.getIndexVersionCreated().onOrAfter(IndexVersions.AGG_METRIC_DOUBLE_REMOVE_POINTS)) { + builder.index(false); + } } NumberFieldMapper fieldMapper = builder.build(context); metricMappers.put(m, fieldMapper); diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapperTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapperTests.java index 3183c55494708..a8229a48e7269 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapperTests.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapperTests.java @@ -6,12 +6,23 @@ */ package org.elasticsearch.xpack.aggregatemetric.mapper; +import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.index.DocValuesSkipIndexType; +import org.apache.lucene.index.IndexableField; import org.apache.lucene.search.FieldExistsQuery; import org.apache.lucene.search.Query; +import org.apache.lucene.util.NumericUtils; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.CheckedConsumer; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.IndexVersion; +import org.elasticsearch.index.IndexVersions; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.DocumentParsingException; +import org.elasticsearch.index.mapper.IndexType; import org.elasticsearch.index.mapper.LuceneDocument; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.Mapper; @@ -20,12 +31,14 @@ import org.elasticsearch.index.mapper.MapperTestCase; import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.index.IndexVersionUtils; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentFactory; import org.elasticsearch.xcontent.json.JsonXContent; import org.elasticsearch.xpack.aggregatemetric.AggregateMetricMapperPlugin; import org.elasticsearch.xpack.aggregatemetric.mapper.AggregateMetricDoubleFieldMapper.Metric; import org.hamcrest.Matchers; +import org.junit.Assert; import org.junit.AssumptionViolatedException; import java.io.IOException; @@ -105,7 +118,12 @@ public void testParseValue() throws Exception { ParsedDocument doc = mapper.parse( source(b -> b.startObject("field").field("min", -10.1).field("max", 50.0).field("value_count", 14).endObject()) ); - assertEquals("DoubleField ", doc.rootDoc().getField("field.min").toString()); + + IndexableField f = doc.rootDoc().getField("field.min"); + assertThat(f, instanceOf(SortedNumericDocValuesField.class)); + SortedNumericDocValuesField sdv = (SortedNumericDocValuesField) f; + assertEquals(NumericUtils.doubleToSortableLong(-10.1), sdv.numericValue()); + assertThat(sdv.fieldType().docValuesSkipIndexType(), equalTo(DocValuesSkipIndexType.NONE)); Mapper fieldMapper = mapper.mappers().getMapper("field"); assertThat(fieldMapper, instanceOf(AggregateMetricDoubleFieldMapper.class)); @@ -621,11 +639,58 @@ protected boolean supportsCopyTo() { @Override protected List getSortShortcutSupport() { - return List.of(new SortShortcutSupport(this::minimalMapping, this::writeField, true)); + Settings settings = Settings.builder().put(IndexSettings.USE_DOC_VALUES_SKIPPER.getKey(), true).build(); + return List.of( + new SortShortcutSupport( + IndexVersion.current(), + settings, + "field", + this::minimalMapping, + b -> {}, + this::writeField, + Assert::assertNotNull + ) + ); } @Override protected boolean supportsDocValuesSkippers() { - return false; + return false; // can't configure `index` so normal test doesn't work + } + + public void testIndexTypes() throws Exception { + { + var indexSettings = getIndexSettingsBuilder().put(IndexSettings.MODE.getKey(), IndexMode.TIME_SERIES.getName()) + .put(IndexMetadata.INDEX_ROUTING_PATH.getKey(), "dimension_field") + .build(); + MapperService mapperService = createMapperService(indexSettings, fieldMapping(this::minimalMapping)); + MappedFieldType ft = mapperService.fieldType("field"); + assertIndexType(ft, IndexType.skippers()); + } + { + var oldVersion = IndexVersionUtils.randomPreviousCompatibleVersion(random(), IndexVersions.AGG_METRIC_DOUBLE_REMOVE_POINTS); + MapperService mapperService = createMapperService(oldVersion, fieldMapping(this::minimalMapping)); + MappedFieldType ft = mapperService.fieldType("field"); + assertIndexType(ft, IndexType.points(true, true)); + } + { + MapperService mapperService = createMapperService( + IndexVersions.AGG_METRIC_DOUBLE_REMOVE_POINTS, + fieldMapping(this::minimalMapping) + ); + MappedFieldType ft = mapperService.fieldType("field"); + assertIndexType(ft, IndexType.docValuesOnly()); + } } + + private void assertIndexType(MappedFieldType ft, IndexType indexType) { + assertThat(ft, instanceOf(AggregateMetricDoubleFieldMapper.AggregateMetricDoubleFieldType.class)); + AggregateMetricDoubleFieldMapper.AggregateMetricDoubleFieldType aft = + (AggregateMetricDoubleFieldMapper.AggregateMetricDoubleFieldType) ft; + assertThat(aft.indexType(), equalTo(indexType)); + for (MappedFieldType subField : aft.getMetricFields().values()) { + assertThat(subField.indexType(), equalTo(indexType)); + } + } + }