Merge pull request #19217 from jimferenczi/mapper_size_docvalues2

Add doc values support to the _size field in the mapper-size plugin

buildSrc/src/main/resources/checkstyle_suppressions.xml

@@ -1200,10 +1200,6 @@
   <suppress files="plugins[/\\]mapper-murmur3[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]murmur3[/\\]Murmur3FieldMapper.java" checks="LineLength" />
   <suppress files="plugins[/\\]mapper-murmur3[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]murmur3[/\\]Murmur3FieldMapperTests.java" checks="LineLength" />
   <suppress files="plugins[/\\]mapper-murmur3[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]murmur3[/\\]Murmur3FieldMapperUpgradeTests.java" checks="LineLength" />
-  <suppress files="plugins[/\\]mapper-size[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]size[/\\]SizeFieldMapper.java" checks="LineLength" />
-  <suppress files="plugins[/\\]mapper-size[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]size[/\\]SizeFieldMapperUpgradeTests.java" checks="LineLength" />
-  <suppress files="plugins[/\\]mapper-size[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]size[/\\]SizeMappingIT.java" checks="LineLength" />
-  <suppress files="plugins[/\\]mapper-size[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]index[/\\]mapper[/\\]size[/\\]SizeMappingTests.java" checks="LineLength" />
   <suppress files="plugins[/\\]repository-azure[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]cloud[/\\]azure[/\\]blobstore[/\\]AzureBlobContainer.java" checks="LineLength" />
   <suppress files="plugins[/\\]repository-azure[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]cloud[/\\]azure[/\\]blobstore[/\\]AzureBlobStore.java" checks="LineLength" />
   <suppress files="plugins[/\\]repository-azure[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]cloud[/\\]azure[/\\]storage[/\\]AzureStorageServiceImpl.java" checks="LineLength" />

docs/plugins/mapper-size.asciidoc

@@ -52,7 +52,8 @@ PUT my_index
 --------------------------
 // CONSOLE
 
-The value of the `_size` field is accessible in queries:
+The value of the `_size` field is accessible in queries, aggregations, scripts,
+and when sorting:
 
 [source,js]
 --------------------------
@@ -75,6 +76,26 @@ GET my_index/_search
         "gt": 10
       }
     }
+  },
+  "aggs": {
+    "sizes": {
+      "terms": {
+        "field": "_size", <2>
+        "size": 10
+      }
+    }
+  },
+  "sort": [
+    {
+      "_size": { <3>
+        "order": "desc"
+      }
+    }
+  ],
+  "script_fields": {
+    "size": {
+      "script": "doc['_size']"  <4>
+    }
   }
 }
 --------------------------

docs/reference/migration/migrate_5_0/plugins.asciidoc

@@ -140,3 +140,11 @@ remove their `onModule(ActionModule)` implementation.
 
 Plugins that register custom `RestHandler`s should implement `ActionPlugin` and
 remove their `onModule(NetworkModule)` implemnetation.
+
+==== Mapper-Size plugin
+
+The metadata field `_size` is not accessible in aggregations, scripts and when
+sorting for indices created in 2.x even if the index has been upgraded using the <<indices-upgrade,`upgrade`>> API.
+If these features are needed in your application it is required to reindex the data with Elasticsearch 5.x.
+The easiest way to reindex old indices is to use the `reindex` API, or the reindex UI provided by
+the <<migration-plugin,Migration Plugin>>.

plugins/mapper-size/src/main/java/org/elasticsearch/index/mapper/size/SizeFieldMapper.java

@@ -42,15 +42,15 @@ import java.util.Map;
 import static org.elasticsearch.common.xcontent.support.XContentMapValues.lenientNodeBooleanValue;
 
 public class SizeFieldMapper extends MetadataFieldMapper {
-
     public static final String NAME = "_size";
-    public static final String CONTENT_TYPE = "_size";
 
     public static class Defaults  {
         public static final EnabledAttributeMapper ENABLED_STATE = EnabledAttributeMapper.UNSET_DISABLED;
 
-        public static final MappedFieldType SIZE_FIELD_TYPE = new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.INTEGER);
+        public static final MappedFieldType SIZE_FIELD_TYPE =
-        public static final MappedFieldType LEGACY_SIZE_FIELD_TYPE = LegacyIntegerFieldMapper.Defaults.FIELD_TYPE.clone();
+            new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.INTEGER);
+        public static final MappedFieldType LEGACY_SIZE_FIELD_TYPE =
+            LegacyIntegerFieldMapper.Defaults.FIELD_TYPE.clone();
 
         static {
             SIZE_FIELD_TYPE.setStored(true);
@@ -68,14 +68,31 @@ public class SizeFieldMapper extends MetadataFieldMapper {
         }
     }
 
+    private static MappedFieldType defaultFieldType(Version indexCreated) {
+        MappedFieldType defaultFieldType;
+        if (indexCreated.before(Version.V_5_0_0_alpha2)) {
+            defaultFieldType = Defaults.LEGACY_SIZE_FIELD_TYPE.clone();
+            // doc_values are disabled for bwc with indices created before V_5_0_0_alpha4
+            defaultFieldType.setHasDocValues(false);
+        } else {
+            defaultFieldType = Defaults.SIZE_FIELD_TYPE.clone();
+            if (indexCreated.onOrBefore(Version.V_5_0_0_alpha4)) {
+                // doc_values are disabled for bwc with indices created before V_5_0_0_alpha4
+                defaultFieldType.setHasDocValues(false);
+            } else {
+                defaultFieldType.setHasDocValues(true);
+            }
+        }
+        return defaultFieldType;
+    }
+
     public static class Builder extends MetadataFieldMapper.Builder<Builder, SizeFieldMapper> {
 
         protected EnabledAttributeMapper enabledState = EnabledAttributeMapper.UNSET_DISABLED;
 
         private Builder(MappedFieldType existing, Version indexCreated) {
-            super(NAME, existing == null
+            super(NAME, existing == null ? defaultFieldType(indexCreated) : existing.clone(),
-                    ? indexCreated.before(Version.V_5_0_0_alpha2) ? Defaults.LEGACY_SIZE_FIELD_TYPE : Defaults.SIZE_FIELD_TYPE
+                defaultFieldType(indexCreated));
-                    : existing, Defaults.LEGACY_SIZE_FIELD_TYPE);
             builder = this;
         }
 
@@ -87,21 +104,27 @@ public class SizeFieldMapper extends MetadataFieldMapper {
         @Override
         public SizeFieldMapper build(BuilderContext context) {
             setupFieldType(context);
+            if (context.indexCreatedVersion().onOrBefore(Version.V_5_0_0_alpha4)) {
+                // Make sure that the doc_values are disabled on indices created before V_5_0_0_alpha4
                 fieldType.setHasDocValues(false);
+            }
             return new SizeFieldMapper(enabledState, fieldType, context.indexSettings());
         }
     }
 
     public static class TypeParser implements MetadataFieldMapper.TypeParser {
         @Override
-        public MetadataFieldMapper.Builder<?, ?> parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
+        public MetadataFieldMapper.Builder<?, ?> parse(String name, Map<String, Object> node,
-            Builder builder = new Builder(parserContext.mapperService().fullName(NAME), parserContext.indexVersionCreated());
+                                                       ParserContext parserContext) throws MapperParsingException {
+            Builder builder = new Builder(parserContext.mapperService().fullName(NAME),
+                parserContext.indexVersionCreated());
             for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
                 Map.Entry<String, Object> entry = iterator.next();
                 String fieldName = entry.getKey();
                 Object fieldNode = entry.getValue();
                 if (fieldName.equals("enabled")) {
-                    builder.enabled(lenientNodeBooleanValue(fieldNode) ? EnabledAttributeMapper.ENABLED : EnabledAttributeMapper.DISABLED);
+                    builder.enabled(lenientNodeBooleanValue(fieldNode) ?
+                        EnabledAttributeMapper.ENABLED : EnabledAttributeMapper.DISABLED);
                     iterator.remove();
                 }
             }
@@ -116,14 +139,15 @@ public class SizeFieldMapper extends MetadataFieldMapper {
 
     private EnabledAttributeMapper enabledState;
 
-    private SizeFieldMapper(Settings indexSettings, MappedFieldType mappedFieldType) {
+    private SizeFieldMapper(Settings indexSettings, MappedFieldType existing) {
-        this(Defaults.ENABLED_STATE, mappedFieldType == null ? Defaults.LEGACY_SIZE_FIELD_TYPE : mappedFieldType, indexSettings);
+        this(Defaults.ENABLED_STATE,
+            existing == null ? defaultFieldType(Version.indexCreated(indexSettings)) : existing.clone(),
+            indexSettings);
     }
 
     private SizeFieldMapper(EnabledAttributeMapper enabled, MappedFieldType fieldType, Settings indexSettings) {
-        super(NAME, fieldType, Defaults.LEGACY_SIZE_FIELD_TYPE, indexSettings);
+        super(NAME, fieldType, defaultFieldType(Version.indexCreated(indexSettings)), indexSettings);
         this.enabledState = enabled;
-
     }
 
     @Override

plugins/mapper-size/src/test/java/org/elasticsearch/index/mapper/size/SizeFieldMapperUpgradeTests.java

@@ -67,7 +67,8 @@ public class SizeFieldMapperUpgradeTests extends ESIntegTestCase {
         Settings settings = Settings.builder()
                 .put(Environment.PATH_DATA_SETTING.getKey(), dataPath)
                 .build();
-        final String node = internalCluster().startDataOnlyNode(settings); // workaround for dangling index loading issue when node is master
+        // workaround for dangling index loading issue when node is master
+        final String node = internalCluster().startDataOnlyNode(settings);
         Path[] nodePaths = internalCluster().getInstance(NodeEnvironment.class, node).nodeDataPaths();
         assertEquals(1, nodePaths.length);
         dataPath = nodePaths[0].resolve(NodeEnvironment.INDICES_FOLDER);

plugins/mapper-size/src/test/java/org/elasticsearch/index/mapper/size/SizeMappingIT.java

@@ -49,15 +49,19 @@ public class SizeMappingIT extends ESIntegTestCase {
         String index = "foo";
         String type = "mytype";
 
-        XContentBuilder builder = jsonBuilder().startObject().startObject("_size").field("enabled", true).endObject().endObject();
+        XContentBuilder builder =
+            jsonBuilder().startObject().startObject("_size").field("enabled", true).endObject().endObject();
         assertAcked(client().admin().indices().prepareCreate(index).addMapping(type, builder));
 
         // check mapping again
         assertSizeMappingEnabled(index, type, true);
 
         // update some field in the mapping
-        XContentBuilder updateMappingBuilder = jsonBuilder().startObject().startObject("properties").startObject("otherField").field("type", "text").endObject().endObject().endObject();
+        XContentBuilder updateMappingBuilder =
-        PutMappingResponse putMappingResponse = client().admin().indices().preparePutMapping(index).setType(type).setSource(updateMappingBuilder).get();
+            jsonBuilder().startObject().startObject("properties").startObject("otherField").field("type", "text")
+                .endObject().endObject().endObject();
+        PutMappingResponse putMappingResponse =
+            client().admin().indices().preparePutMapping(index).setType(type).setSource(updateMappingBuilder).get();
         assertAcked(putMappingResponse);
 
         // make sure size field is still in mapping
@@ -68,15 +72,18 @@ public class SizeMappingIT extends ESIntegTestCase {
         String index = "foo";
         String type = "mytype";
 
-        XContentBuilder builder = jsonBuilder().startObject().startObject("_size").field("enabled", true).endObject().endObject();
+        XContentBuilder builder =
+            jsonBuilder().startObject().startObject("_size").field("enabled", true).endObject().endObject();
         assertAcked(client().admin().indices().prepareCreate(index).addMapping(type, builder));
 
         // check mapping again
         assertSizeMappingEnabled(index, type, true);
 
         // update some field in the mapping
-        XContentBuilder updateMappingBuilder = jsonBuilder().startObject().startObject("_size").field("enabled", false).endObject().endObject();
+        XContentBuilder updateMappingBuilder =
-        PutMappingResponse putMappingResponse = client().admin().indices().preparePutMapping(index).setType(type).setSource(updateMappingBuilder).get();
+            jsonBuilder().startObject().startObject("_size").field("enabled", false).endObject().endObject();
+        PutMappingResponse putMappingResponse =
+            client().admin().indices().preparePutMapping(index).setType(type).setSource(updateMappingBuilder).get();
         assertAcked(putMappingResponse);
 
         // make sure size field is still in mapping
@@ -84,8 +91,10 @@ public class SizeMappingIT extends ESIntegTestCase {
     }
 
     private void assertSizeMappingEnabled(String index, String type, boolean enabled) throws IOException {
-        String errMsg = String.format(Locale.ROOT, "Expected size field mapping to be " + (enabled ? "enabled" : "disabled") + " for %s/%s", index, type);
+        String errMsg = String.format(Locale.ROOT,
-        GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes(type).get();
+            "Expected size field mapping to be " + (enabled ? "enabled" : "disabled") + " for %s/%s", index, type);
+        GetMappingsResponse getMappingsResponse =
+            client().admin().indices().prepareGetMappings(index).addTypes(type).get();
         Map<String, Object> mappingSource = getMappingsResponse.getMappings().get(index).get(type).getSourceAsMap();
         assertThat(errMsg, mappingSource, hasKey("_size"));
         String sizeAsString = mappingSource.get("_size").toString();

plugins/mapper-size/src/test/java/org/elasticsearch/index/mapper/size/SizeMappingTests.java

@@ -19,52 +19,42 @@
 
 package org.elasticsearch.index.mapper.size;
 
-import java.util.Collections;
+import java.util.Collection;
-import java.util.Map;
 
+import org.elasticsearch.Version;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.compress.CompressedXContent;
-import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.IndexService;
-import org.elasticsearch.index.mapper.DocumentMapper;
-import org.elasticsearch.index.mapper.DocumentMapperParser;
-import org.elasticsearch.index.mapper.Mapper;
 import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.ParsedDocument;
 import org.elasticsearch.index.mapper.SourceToParse;
-import org.elasticsearch.indices.IndicesModule;
+import org.elasticsearch.index.mapper.MappedFieldType;
-import org.elasticsearch.plugins.MapperPlugin;
+import org.elasticsearch.index.mapper.core.LegacyNumberFieldMapper;
+import org.elasticsearch.index.mapper.core.NumberFieldMapper;
+import org.elasticsearch.plugin.mapper.MapperSizePlugin;
+import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESSingleNodeTestCase;
-import org.junit.Before;
+import org.elasticsearch.test.InternalSettingsPlugin;
-
 
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
+import static org.hamcrest.Matchers.instanceOf;
 
 import org.apache.lucene.index.IndexableField;
 
 public class SizeMappingTests extends ESSingleNodeTestCase {
-
+    @Override
-    IndexService indexService;
+    protected Collection<Class<? extends Plugin>> getPlugins() {
-    MapperService mapperService;
+        return pluginList(MapperSizePlugin.class, InternalSettingsPlugin.class);
-    DocumentMapperParser parser;
-
-    @Before
-    public void before() {
-        indexService = createIndex("test");
-        IndicesModule indices = newTestIndicesModule(Collections.emptyMap(),
-            Collections.singletonMap(SizeFieldMapper.NAME, new SizeFieldMapper.TypeParser())
-        );
-        mapperService = new MapperService(indexService.getIndexSettings(), indexService.analysisService(), indexService.similarityService(), indices.getMapperRegistry(), indexService::newQueryShardContext);
-        parser = mapperService.documentMapperParser();
     }
 
     public void testSizeEnabled() throws Exception {
-        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+        IndexService service = createIndex("test", Settings.EMPTY, "type", "_size", "enabled=true");
-                .startObject("_size").field("enabled", true).endObject()
+        DocumentMapper docMapper = service.mapperService().documentMapper("type");
-                .endObject().endObject().string();
-        DocumentMapper docMapper = parser.parse("type", new CompressedXContent(mapping));
 
         BytesReference source = XContentFactory.jsonBuilder()
             .startObject()
@@ -84,10 +74,8 @@ public class SizeMappingTests extends ESSingleNodeTestCase {
     }
 
     public void testSizeDisabled() throws Exception {
-        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+        IndexService service = createIndex("test", Settings.EMPTY, "type", "_size", "enabled=false");
-                .startObject("_size").field("enabled", false).endObject()
+        DocumentMapper docMapper = service.mapperService().documentMapper("type");
-                .endObject().endObject().string();
-        DocumentMapper docMapper = parser.parse("type", new CompressedXContent(mapping));
 
         BytesReference source = XContentFactory.jsonBuilder()
             .startObject()
@@ -100,9 +88,8 @@ public class SizeMappingTests extends ESSingleNodeTestCase {
     }
 
     public void testSizeNotSet() throws Exception {
-        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+        IndexService service = createIndex("test", Settings.EMPTY, "type");
-                .endObject().endObject().string();
+        DocumentMapper docMapper = service.mapperService().documentMapper("type");
-        DocumentMapper docMapper = parser.parse("type", new CompressedXContent(mapping));
 
         BytesReference source = XContentFactory.jsonBuilder()
             .startObject()
@@ -115,16 +102,54 @@ public class SizeMappingTests extends ESSingleNodeTestCase {
     }
 
     public void testThatDisablingWorksWhenMerging() throws Exception {
-        String enabledMapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+        IndexService service = createIndex("test", Settings.EMPTY, "type", "_size", "enabled=true");
-                .startObject("_size").field("enabled", true).endObject()
+        DocumentMapper docMapper = service.mapperService().documentMapper("type");
-                .endObject().endObject().string();
+        assertThat(docMapper.metadataMapper(SizeFieldMapper.class).enabled(), is(true));
-        DocumentMapper enabledMapper = mapperService.merge("type", new CompressedXContent(enabledMapping), MapperService.MergeReason.MAPPING_UPDATE, false);
 
         String disabledMapping = XContentFactory.jsonBuilder().startObject().startObject("type")
             .startObject("_size").field("enabled", false).endObject()
             .endObject().endObject().string();
-        DocumentMapper disabledMapper = mapperService.merge("type", new CompressedXContent(disabledMapping), MapperService.MergeReason.MAPPING_UPDATE, false);
+        docMapper = service.mapperService().merge("type", new CompressedXContent(disabledMapping),
+            MapperService.MergeReason.MAPPING_UPDATE, false);
 
-        assertThat(disabledMapper.metadataMapper(SizeFieldMapper.class).enabled(), is(false));
+        assertThat(docMapper.metadataMapper(SizeFieldMapper.class).enabled(), is(false));
     }
+
+    public void testBWCMapper() throws Exception {
+        {
+            // IntPoint && docvalues=true for V_5_0_0_alpha5
+            IndexService service = createIndex("foo", Settings.EMPTY, "bar", "_size", "enabled=true");
+            DocumentMapper docMapper = service.mapperService().documentMapper("bar");
+            SizeFieldMapper mapper = docMapper.metadataMapper(SizeFieldMapper.class);
+            assertThat(mapper.enabled(), is(true));
+            MappedFieldType ft = mapper.fieldType();
+            assertThat(ft.hasDocValues(), is(true));
+            assertThat(mapper.fieldType(), instanceOf(NumberFieldMapper.NumberFieldType.class));
+        }
+
+        {
+            // IntPoint with docvalues=false if version > V_5_0_0_alpha2 && version < V_5_0_0_beta1
+            IndexService service = createIndex("foo2",
+                Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_5_0_0_alpha4.id).build(),
+                "bar", "_size", "enabled=true");
+            DocumentMapper docMapper = service.mapperService().documentMapper("bar");
+            SizeFieldMapper mapper = docMapper.metadataMapper(SizeFieldMapper.class);
+            assertThat(mapper.enabled(), is(true));
+            assertThat(mapper.fieldType().hasDocValues(), is(false));
+            assertThat(mapper.fieldType(), instanceOf(NumberFieldMapper.NumberFieldType.class));
+        }
+
+        {
+            // LegacyIntField with docvalues=false if version < V_5_0_0_alpha2
+            IndexService service = createIndex("foo3",
+                Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.V_5_0_0_alpha1.id).build(),
+                "bar", "_size", "enabled=true");
+            DocumentMapper docMapper = service.mapperService().documentMapper("bar");
+            SizeFieldMapper mapper = docMapper.metadataMapper(SizeFieldMapper.class);
+            assertThat(mapper.enabled(), is(true));
+            assertThat(mapper.fieldType().hasDocValues(), is(false));
+            assertThat(mapper.fieldType(), instanceOf(LegacyNumberFieldMapper.NumberFieldType.class));
+        }
+    }
+
 }