Deprecate string in favor of text/keyword. #16877

This commit removes the ability to use string fields on indices created on or
after 5.0. Dynamic mappings now generate text fields by default for strings
but there are plans to also add a sub keyword field (in a future PR).

Most of the changes in this commit are just about replacing string with
keyword or text. Some tests have been removed because they existed because of
corner cases of string mappings like setting ignore-above on a text field or
enabling term vectors on a keyword field which are now impossible.

The plan is to remove strings entirely in 6.0.

core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

@@ -510,17 +510,17 @@ private static void parseNullValue(ParseContext context, ObjectMapper parentMapp
     private static Mapper.Builder<?,?> createBuilderFromFieldType(final ParseContext context, MappedFieldType fieldType, String currentFieldName) {
         Mapper.Builder builder = null;
         if (fieldType instanceof StringFieldType) {
-            builder = context.root().findTemplateBuilder(context, currentFieldName, "string");
+            builder = context.root().findTemplateBuilder(context, currentFieldName, "string", "string");
             if (builder == null) {
                 builder = new StringFieldMapper.Builder(currentFieldName);
             }
         } else if (fieldType instanceof TextFieldType) {
-            builder = context.root().findTemplateBuilder(context, currentFieldName, "string");
+            builder = context.root().findTemplateBuilder(context, currentFieldName, "text", "string");
             if (builder == null) {
                 builder = new TextFieldMapper.Builder(currentFieldName);
             }
         } else if (fieldType instanceof KeywordFieldType) {
-            builder = context.root().findTemplateBuilder(context, currentFieldName, "string");
+            builder = context.root().findTemplateBuilder(context, currentFieldName, "keyword", "string");
             if (builder == null) {
                 builder = new KeywordFieldMapper.Builder(currentFieldName);
             }
@@ -568,7 +568,7 @@ private static Mapper.Builder<?,?> createBuilderFromDynamicValue(final ParseCont
             // we need to do it here so we can handle things like attachment templates, where calling
             // text (to see if its a date) causes the binary value to be cleared
             {
-                Mapper.Builder builder = context.root().findTemplateBuilder(context, currentFieldName, "string", null);
+                Mapper.Builder builder = context.root().findTemplateBuilder(context, currentFieldName, "text", null);
                 if (builder != null) {
                     return builder;
                 }
@@ -617,7 +617,7 @@ private static Mapper.Builder<?,?> createBuilderFromDynamicValue(final ParseCont
             }
             Mapper.Builder builder = context.root().findTemplateBuilder(context, currentFieldName, "string");
             if (builder == null) {
-                builder = new StringFieldMapper.Builder(currentFieldName);
+                builder = new TextFieldMapper.Builder(currentFieldName);
             }
             return builder;
         } else if (token == XContentParser.Token.VALUE_NUMBER) {

core/src/main/java/org/elasticsearch/index/mapper/core/StringFieldMapper.java

@@ -132,6 +132,10 @@ public StringFieldMapper build(BuilderContext context) {
     public static class TypeParser implements Mapper.TypeParser {
         @Override
         public Mapper.Builder parse(String fieldName, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
+            if (parserContext.indexVersionCreated().onOrAfter(Version.V_5_0_0)) {
+                throw new IllegalArgumentException("The [string] type is removed in 5.0. You should now use either a [text] "
+                        + "or [keyword] field instead for field [" + fieldName + "]");
+            }
             StringFieldMapper.Builder builder = new StringFieldMapper.Builder(fieldName);
             // hack for the fact that string can't just accept true/false for
             // the index property and still accepts no/not_analyzed/analyzed
@@ -236,6 +240,10 @@ protected StringFieldMapper(String simpleName, MappedFieldType fieldType, Mapped
                                 int positionIncrementGap, int ignoreAbove,
                                 Settings indexSettings, MultiFields multiFields, CopyTo copyTo) {
         super(simpleName, fieldType, defaultFieldType, indexSettings, multiFields, copyTo);
+        if (Version.indexCreated(indexSettings).onOrAfter(Version.V_5_0_0)) {
+            throw new IllegalArgumentException("The [string] type is removed in 5.0. You should now use either a [text] "
+                    + "or [keyword] field instead for field [" + fieldType.name() + "]");
+        }
         if (fieldType.tokenized() && fieldType.indexOptions() != NONE && fieldType().hasDocValues()) {
             throw new MapperParsingException("Field [" + fieldType.name() + "] cannot be analyzed and have doc values");
         }

core/src/main/java/org/elasticsearch/index/mapper/geo/BaseGeoPointFieldMapper.java

@@ -20,8 +20,6 @@
 package org.elasticsearch.index.mapper.geo;
 
 import org.apache.lucene.document.Field;
-import org.apache.lucene.index.IndexOptions;
-import org.apache.lucene.spatial.geopoint.document.GeoPointField;
 import org.apache.lucene.spatial.util.GeoHashUtils;
 import org.apache.lucene.util.NumericUtils;
 import org.elasticsearch.Version;
@@ -40,9 +38,8 @@
 import org.elasticsearch.index.mapper.MapperParsingException;
 import org.elasticsearch.index.mapper.ParseContext;
 import org.elasticsearch.index.mapper.core.DoubleFieldMapper;
+import org.elasticsearch.index.mapper.core.KeywordFieldMapper;
 import org.elasticsearch.index.mapper.core.NumberFieldMapper;
-import org.elasticsearch.index.mapper.core.StringFieldMapper;
-import org.elasticsearch.index.mapper.core.TokenCountFieldMapper;
 import org.elasticsearch.index.mapper.object.ArrayValueMapperParser;
 
 import java.io.IOException;
@@ -148,7 +145,7 @@ protected Explicit<Boolean> ignoreMalformed(BuilderContext context) {
 
         public abstract Y build(BuilderContext context, String simpleName, MappedFieldType fieldType, MappedFieldType defaultFieldType,
                                 Settings indexSettings, DoubleFieldMapper latMapper, DoubleFieldMapper lonMapper,
-                                StringFieldMapper geoHashMapper, MultiFields multiFields, Explicit<Boolean> ignoreMalformed, CopyTo copyTo);
+                                KeywordFieldMapper geoHashMapper, MultiFields multiFields, Explicit<Boolean> ignoreMalformed, CopyTo copyTo);
 
         public Y build(Mapper.BuilderContext context) {
             GeoPointFieldType geoPointFieldType = (GeoPointFieldType)fieldType;
@@ -168,11 +165,10 @@ public Y build(Mapper.BuilderContext context) {
                 lonMapper = (DoubleFieldMapper) lonMapperBuilder.includeInAll(false).store(fieldType.stored()).docValues(false).build(context);
                 geoPointFieldType.setLatLonEnabled(latMapper.fieldType(), lonMapper.fieldType());
             }
-            StringFieldMapper geoHashMapper = null;
+            KeywordFieldMapper geoHashMapper = null;
             if (enableGeoHash || enableGeoHashPrefix) {
                 // TODO: possible also implicitly enable geohash if geohash precision is set
-                geoHashMapper = new StringFieldMapper.Builder(Names.GEOHASH).index(true).tokenized(false).includeInAll(false).store(fieldType.stored())
-                        .omitNorms(true).indexOptions(IndexOptions.DOCS).build(context);
+                geoHashMapper = new KeywordFieldMapper.Builder(Names.GEOHASH).index(true).includeInAll(false).store(fieldType.stored()).build(context);
                 geoPointFieldType.setGeoHashEnabled(geoHashMapper.fieldType(), geoHashPrecision, enableGeoHashPrefix);
             }
             context.path().remove();
@@ -349,12 +345,12 @@ public void setLatLonEnabled(MappedFieldType latFieldType, MappedFieldType lonFi
 
     protected DoubleFieldMapper lonMapper;
 
-    protected StringFieldMapper geoHashMapper;
+    protected KeywordFieldMapper geoHashMapper;
 
     protected Explicit<Boolean> ignoreMalformed;
 
     protected BaseGeoPointFieldMapper(String simpleName, MappedFieldType fieldType, MappedFieldType defaultFieldType, Settings indexSettings,
-                                      DoubleFieldMapper latMapper, DoubleFieldMapper lonMapper, StringFieldMapper geoHashMapper,
+                                      DoubleFieldMapper latMapper, DoubleFieldMapper lonMapper, KeywordFieldMapper geoHashMapper,
                                       MultiFields multiFields, Explicit<Boolean> ignoreMalformed, CopyTo copyTo) {
         super(simpleName, fieldType, defaultFieldType, indexSettings, multiFields, copyTo);
         this.latMapper = latMapper;
@@ -507,7 +503,7 @@ protected void doXContentBody(XContentBuilder builder, boolean includeDefaults,
     @Override
     public FieldMapper updateFieldType(Map<String, MappedFieldType> fullNameToFieldType) {
         BaseGeoPointFieldMapper updated = (BaseGeoPointFieldMapper) super.updateFieldType(fullNameToFieldType);
-        StringFieldMapper geoUpdated = geoHashMapper == null ? null : (StringFieldMapper) geoHashMapper.updateFieldType(fullNameToFieldType);
+        KeywordFieldMapper geoUpdated = geoHashMapper == null ? null : (KeywordFieldMapper) geoHashMapper.updateFieldType(fullNameToFieldType);
         DoubleFieldMapper latUpdated = latMapper == null ? null : (DoubleFieldMapper) latMapper.updateFieldType(fullNameToFieldType);
         DoubleFieldMapper lonUpdated = lonMapper == null ? null : (DoubleFieldMapper) lonMapper.updateFieldType(fullNameToFieldType);
         if (updated == this

core/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java

@@ -33,7 +33,7 @@
 import org.elasticsearch.index.mapper.MapperParsingException;
 import org.elasticsearch.index.mapper.ParseContext;
 import org.elasticsearch.index.mapper.core.DoubleFieldMapper;
-import org.elasticsearch.index.mapper.core.StringFieldMapper;
+import org.elasticsearch.index.mapper.core.KeywordFieldMapper;
 
 import java.io.IOException;
 import java.util.Map;
@@ -79,7 +79,7 @@ public Builder(String name) {
         @Override
         public GeoPointFieldMapper build(BuilderContext context, String simpleName, MappedFieldType fieldType,
                                          MappedFieldType defaultFieldType, Settings indexSettings, DoubleFieldMapper latMapper,
-                                         DoubleFieldMapper lonMapper, StringFieldMapper geoHashMapper, MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
+                                         DoubleFieldMapper lonMapper, KeywordFieldMapper geoHashMapper, MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
                                          CopyTo copyTo) {
             fieldType.setTokenized(false);
             if (context.indexCreatedVersion().before(Version.V_2_3_0)) {
@@ -110,7 +110,7 @@ public static class TypeParser extends BaseGeoPointFieldMapper.TypeParser {
 
     public GeoPointFieldMapper(String simpleName, MappedFieldType fieldType, MappedFieldType defaultFieldType, Settings indexSettings,
                                DoubleFieldMapper latMapper, DoubleFieldMapper lonMapper,
-                               StringFieldMapper geoHashMapper, MultiFields multiFields, Explicit<Boolean> ignoreMalformed, CopyTo copyTo) {
+                               KeywordFieldMapper geoHashMapper, MultiFields multiFields, Explicit<Boolean> ignoreMalformed, CopyTo copyTo) {
         super(simpleName, fieldType, defaultFieldType, indexSettings, latMapper, lonMapper, geoHashMapper, multiFields,
                 ignoreMalformed, copyTo);
     }

core/src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapperLegacy.java

@@ -24,7 +24,6 @@
 import org.apache.lucene.document.Field;
 import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.util.BytesRef;
-import org.elasticsearch.Version;
 import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.geo.GeoDistance;
@@ -40,8 +39,8 @@
 import org.elasticsearch.index.mapper.MapperParsingException;
 import org.elasticsearch.index.mapper.ParseContext;
 import org.elasticsearch.index.mapper.core.DoubleFieldMapper;
+import org.elasticsearch.index.mapper.core.KeywordFieldMapper;
 import org.elasticsearch.index.mapper.core.NumberFieldMapper.CustomNumericDocValuesField;
-import org.elasticsearch.index.mapper.core.StringFieldMapper;
 import org.elasticsearch.index.mapper.object.ArrayValueMapperParser;
 
 import java.io.IOException;
@@ -110,7 +109,7 @@ protected Explicit<Boolean> coerce(BuilderContext context) {
         @Override
         public GeoPointFieldMapperLegacy build(BuilderContext context, String simpleName, MappedFieldType fieldType,
                                                MappedFieldType defaultFieldType, Settings indexSettings, DoubleFieldMapper latMapper,
-                                               DoubleFieldMapper lonMapper, StringFieldMapper geoHashMapper, MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
+                                               DoubleFieldMapper lonMapper, KeywordFieldMapper geoHashMapper, MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
                                                CopyTo copyTo) {
             fieldType.setTokenized(false);
             setupFieldType(context);
@@ -268,7 +267,7 @@ public GeoPoint decode(long latBits, long lonBits, GeoPoint out) {
 
     public GeoPointFieldMapperLegacy(String simpleName, MappedFieldType fieldType, MappedFieldType defaultFieldType, Settings indexSettings,
                                      DoubleFieldMapper latMapper, DoubleFieldMapper lonMapper,
-                                     StringFieldMapper geoHashMapper, MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
+                                     KeywordFieldMapper geoHashMapper, MultiFields multiFields, Explicit<Boolean> ignoreMalformed,
                                      Explicit<Boolean> coerce, CopyTo copyTo) {
         super(simpleName, fieldType, defaultFieldType, indexSettings, latMapper, lonMapper, geoHashMapper, multiFields,
                 ignoreMalformed, copyTo);

core/src/main/java/org/elasticsearch/index/mapper/object/RootObjectMapper.java

@@ -235,10 +235,30 @@ public FormatDateTimeFormatter[] dynamicDateTimeFormatters() {
         return dynamicDateTimeFormatters;
     }
 
-    public Mapper.Builder findTemplateBuilder(ParseContext context, String name, String dynamicType) {
-        return findTemplateBuilder(context, name, dynamicType, dynamicType);
+    public Mapper.Builder findTemplateBuilder(ParseContext context, String name, String matchType) {
+        final String dynamicType;
+        switch (matchType) {
+        case "string":
+            // string is a corner case since a json string can either map to a
+            // text or keyword field in elasticsearch. For now we use text when
+            // unspecified. For other types, the mapping type matches the json
+            // type so we are fine
+            dynamicType = "text";
+            break;
+        default:
+            dynamicType = matchType;
+            break;
+        }
+        return findTemplateBuilder(context, name, dynamicType, matchType);
     }
 
+    /**
+     * Find a template. Returns {@code null} if no template could be found.
+     * @param name        the field name
+     * @param dynamicType the field type to give the field if the template does not define one
+     * @param matchType   the type of the field in the json document or null if unknown
+     * @return a mapper builder, or null if there is no template for such a field
+     */
     public Mapper.Builder findTemplateBuilder(ParseContext context, String name, String dynamicType, String matchType) {
         DynamicTemplate dynamicTemplate = findTemplate(context.path(), name, matchType);
         if (dynamicTemplate == null) {

core/src/test/java/org/elasticsearch/action/termvectors/AbstractTermVectorsTestCase.java

@@ -82,7 +82,7 @@ public TestFieldSetting(String name, boolean storedOffset, boolean storedPayload
 
         public void addToMappings(XContentBuilder mappingsBuilder) throws IOException {
             mappingsBuilder.startObject(name);
-            mappingsBuilder.field("type", "string");
+            mappingsBuilder.field("type", "text");
             String tv_settings;
             if (storedPositions && storedOffset && storedPayloads) {
                 tv_settings = "with_positions_offsets_payloads";

core/src/test/java/org/elasticsearch/cluster/SpecificMasterNodesIT.java

@@ -129,7 +129,7 @@ public void testAliasFilterValidation() throws Exception {
         logger.info("--> start data node / non master node");
         internalCluster().startNode(settingsBuilder().put(Node.NODE_DATA_SETTING.getKey(), true).put(Node.NODE_MASTER_SETTING.getKey(), false));
 
-        assertAcked(prepareCreate("test").addMapping("type1", "{\"type1\" : {\"properties\" : {\"table_a\" : { \"type\" : \"nested\", \"properties\" : {\"field_a\" : { \"type\" : \"string\" },\"field_b\" :{ \"type\" : \"string\" }}}}}}"));
+        assertAcked(prepareCreate("test").addMapping("type1", "{\"type1\" : {\"properties\" : {\"table_a\" : { \"type\" : \"nested\", \"properties\" : {\"field_a\" : { \"type\" : \"keyword\" },\"field_b\" :{ \"type\" : \"keyword\" }}}}}}"));
         client().admin().indices().prepareAliases().addAlias("test", "a_test", QueryBuilders.nestedQuery("table_a", QueryBuilders.termQuery("table_a.field_b", "y"))).get();
     }
 }

core/src/test/java/org/elasticsearch/codecs/CodecTests.java

@@ -48,7 +48,7 @@ protected Collection<Class<? extends Plugin>> getPlugins() {
 
     public void testAcceptPostingsFormat() throws IOException {
         String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
-                .startObject("properties").startObject("field").field("type", "string").field("postings_format", Codec.getDefault().postingsFormat().getName()).endObject().endObject()
+                .startObject("properties").startObject("field").field("type", "keyword").field("postings_format", Codec.getDefault().postingsFormat().getName()).endObject().endObject()
                 .endObject().endObject().string();
         int i = 0;
         for (Version v : VersionUtils.allVersions()) {
@@ -75,7 +75,7 @@ public void testAcceptPostingsFormat() throws IOException {
 
     public void testAcceptDocValuesFormat() throws IOException {
         String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
-                .startObject("properties").startObject("field").field("type", "string").field("doc_values_format", Codec.getDefault().docValuesFormat().getName()).endObject().endObject()
+                .startObject("properties").startObject("field").field("type", "keyword").field("doc_values_format", Codec.getDefault().docValuesFormat().getName()).endObject().endObject()
                 .endObject().endObject().string();
         int i = 0;
         for (Version v : VersionUtils.allVersions()) {

core/src/test/java/org/elasticsearch/get/GetActionIT.java

@@ -751,7 +751,7 @@ public void testGetFieldsComplexField() throws Exception {
                         .startObject("field1").field("type", "object").startObject("properties")
                         .startObject("field2").field("type", "object").startObject("properties")
                                 .startObject("field3").field("type", "object").startObject("properties")
-                                    .startObject("field4").field("type", "string").field("store", true)
+                                    .startObject("field4").field("type", "text").field("store", true)
                                 .endObject().endObject()
                             .endObject().endObject()
                         .endObject().endObject()

core/src/test/java/org/elasticsearch/index/fielddata/AbstractFieldDataTestCase.java

@@ -135,7 +135,7 @@ public <IFD extends IndexFieldData<?>> IFD getForField(FieldDataType type, Strin
 
     @Before
     public void setup() throws Exception {
-        Version version = VersionUtils.randomVersionBetween(random(), Version.V_2_0_0, Version.CURRENT);
+        Version version = VersionUtils.randomVersionBetween(random(), Version.V_2_0_0, Version.V_2_3_0); // we need 2.x so that fielddata is allowed on string fields
         Settings settings = Settings.builder().put("index.fielddata.cache", "none")
                 .put(IndexMetaData.SETTING_VERSION_CREATED, version).build();
         indexService = createIndex("test", settings);