Browse Source

Disallow `_field_names` enabled setting (#46681)

After deprecating the `enabled` setting for `_field_names`starting with 7.5, 
this change disallows the setting on new indices in 8.0. The setting continues
to work for indices created in 7.x where we continue emitting a deprecation warning.

Relates to #42854
Christoph Büscher 6 years ago
parent
commit
2351aa3efb

+ 5 - 19
docs/reference/mapping/fields/field-names-field.asciidoc

@@ -14,23 +14,9 @@ be available but will not use the `_field_names` field.
 [[disable-field-names]]
 ==== Disabling `_field_names`
 
-NOTE: Disabling `_field_names` has been deprecated and will be removed in a future major version.
+Disabling `_field_names` is no longer possible. It is now enabled by default
+because it no longer carries the index overhead it once did.
 
-Disabling `_field_names` is usually not necessary because it no longer
-carries the index overhead it once did. If you have a lot of fields
-which have `doc_values` and `norms` disabled and you do not need to
-execute `exists` queries using those fields you might want to disable
-`_field_names` by adding the following to the mappings:
-
-[source,console]
---------------------------------------------------
-PUT tweets
-{
-  "mappings": {
-    "_field_names": {
-      "enabled": false
-    }
-  }
-}
---------------------------------------------------
-// TEST[warning:Index [tweets] uses the deprecated `enabled` setting for `_field_names`. Disabling _field_names is not necessary because it no longer carries a large index overhead. Support for this setting will be removed in a future major version. Please remove it from your mappings and templates.]
+NOTE: Support for disabling `_field_names` has been removed. Using it on new
+indices will throw an error. Using it in pre-8.0 indices is still allowed but
+issues a deprecation warning.

+ 10 - 1
docs/reference/migration/migrate_8_0/mappings.asciidoc

@@ -22,4 +22,13 @@ Previously, it was possible to define a multi-field within a multi-field.
 Defining chained multi-fields was deprecated in 7.3 and is now no longer
 supported. To migrate the mappings, all instances of `fields` that occur within
 a `fields` block should be removed, either by flattening the chained `fields`
-blocks into a single level, or by switching to `copy_to` if appropriate.
+blocks into a single level, or by switching to `copy_to` if appropriate.
+
+[float]
+[[fieldnames-enabling]]
+==== Disallow use of the `enabled` setting on the  `_field_names` field
+
+The setting has been deprecated with 7.5 and is no longer supported on new indices.
+Mappings for older indices will continue to work but emit a deprecation warning.
+The `enabled` setting for `_field_names` should be removed from templates and mappings. 
+Disabling _field_names is not necessary because it no longer carries a large index overhead.

+ 9 - 4
server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java

@@ -24,6 +24,7 @@ import org.apache.lucene.document.Field;
 import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.search.Query;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.settings.Settings;
@@ -47,8 +48,7 @@ import java.util.Objects;
  */
 public class FieldNamesFieldMapper extends MetadataFieldMapper {
 
-    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(
-            LogManager.getLogger(FieldNamesFieldMapper.class));
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(FieldNamesFieldMapper.class));
 
     public static final String NAME = "_field_names";
 
@@ -111,8 +111,13 @@ public class FieldNamesFieldMapper extends MetadataFieldMapper {
                 Object fieldNode = entry.getValue();
                 if (fieldName.equals("enabled")) {
                     String indexName = parserContext.mapperService().index().getName();
-                    deprecationLogger.deprecatedAndMaybeLog("field_names_enabled_parameter", ENABLED_DEPRECATION_MESSAGE, indexName);
-                    builder.enabled(XContentMapValues.nodeBooleanValue(fieldNode, name + ".enabled"));
+                    if (parserContext.indexVersionCreated().onOrAfter(Version.V_8_0_0)) {
+                        throw new MapperParsingException("The `enabled` setting for the `_field_names` field has been deprecated and "
+                                + "removed but is still used in index [{}]. Please remove it from your mappings and templates.");
+                    } else {
+                        deprecationLogger.deprecatedAndMaybeLog("field_names_enabled_parameter", ENABLED_DEPRECATION_MESSAGE, indexName);
+                        builder.enabled(XContentMapValues.nodeBooleanValue(fieldNode, name + ".enabled"));
+                    }
                     iterator.remove();
                 }
             }

+ 36 - 18
server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java

@@ -20,12 +20,16 @@
 package org.elasticsearch.index.mapper;
 
 import org.apache.lucene.index.IndexOptions;
+import org.elasticsearch.Version;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.compress.CompressedXContent;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.test.ESSingleNodeTestCase;
+import org.elasticsearch.test.VersionUtils;
 
 import java.util.Arrays;
 import java.util.Collections;
@@ -95,33 +99,33 @@ public class FieldNamesFieldMapperTests extends ESSingleNodeTestCase {
         assertFieldNames(Collections.emptySet(), doc);
     }
 
-    public void testExplicitEnabled() throws Exception {
+    public void testUsingEnabledSettingThrows() throws Exception {
         String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
             .startObject("_field_names").field("enabled", true).endObject()
             .startObject("properties")
             .startObject("field").field("type", "keyword").field("doc_values", false).endObject()
             .endObject().endObject().endObject());
-        DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser()
-            .parse("type", new CompressedXContent(mapping));
-        FieldNamesFieldMapper fieldNamesMapper = docMapper.metadataMapper(FieldNamesFieldMapper.class);
-        assertTrue(fieldNamesMapper.fieldType().isEnabled());
+        MapperParsingException ex = expectThrows(MapperParsingException.class,
+                () -> createIndex("test").mapperService().documentMapperParser().parse("type", new CompressedXContent(mapping)));
 
-        ParsedDocument doc = docMapper.parse(new SourceToParse("test", "type", "1",
-            BytesReference.bytes(XContentFactory.jsonBuilder()
-                .startObject()
-                .field("field", "value")
-                .endObject()),
-            XContentType.JSON));
-
-        assertFieldNames(set("field"), doc);
-        assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test"));
+        assertEquals("The `enabled` setting for the `_field_names` field has been deprecated and removed but is still used in index [{}]. "
+                + "Please remove it from your mappings and templates.", ex.getMessage());
     }
 
-    public void testDisabled() throws Exception {
+    /**
+     * disabling the _field_names should still work for indices before 8.0
+     */
+    public void testUsingEnabledBefore8() throws Exception {
         String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
             .startObject("_field_names").field("enabled", false).endObject()
             .endObject().endObject());
-        DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser()
+
+        DocumentMapper docMapper = createIndex("test",
+                Settings.builder()
+                        .put(IndexMetaData.SETTING_INDEX_VERSION_CREATED.getKey(),
+                                VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0))
+                        .build()).mapperService()
+                        .documentMapperParser()
             .parse("type", new CompressedXContent(mapping));
         FieldNamesFieldMapper fieldNamesMapper = docMapper.metadataMapper(FieldNamesFieldMapper.class);
         assertFalse(fieldNamesMapper.fieldType().isEnabled());
@@ -137,14 +141,20 @@ public class FieldNamesFieldMapperTests extends ESSingleNodeTestCase {
         assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test"));
     }
 
-    public void testMergingMappings() throws Exception {
+    /**
+     * Merging the "_field_names" enabled setting is forbidden in 8.0, but we still want to tests the behavior on pre-8 indices
+     */
+    public void testMergingMappingsBefore8() throws Exception {
         String enabledMapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
             .startObject("_field_names").field("enabled", true).endObject()
             .endObject().endObject());
         String disabledMapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
             .startObject("_field_names").field("enabled", false).endObject()
             .endObject().endObject());
-        MapperService mapperService = createIndex("test").mapperService();
+        MapperService mapperService = createIndex("test", Settings.builder()
+                .put(IndexMetaData.SETTING_INDEX_VERSION_CREATED.getKey(),
+                        VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0))
+                .build()).mapperService();
 
         DocumentMapper mapperEnabled = mapperService.merge("type", new CompressedXContent(enabledMapping),
             MapperService.MergeReason.MAPPING_UPDATE);
@@ -156,4 +166,12 @@ public class FieldNamesFieldMapperTests extends ESSingleNodeTestCase {
         assertTrue(mapperEnabled.metadataMapper(FieldNamesFieldMapper.class).fieldType().isEnabled());
         assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test"));
     }
+
+    @Override
+    protected boolean forbidPrivateIndexSettings() {
+        /**
+         * This is needed to force the index version with {@link IndexMetaData.SETTING_INDEX_VERSION_CREATED}.
+         */
+        return false;
+    }
 }

+ 0 - 32
server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java

@@ -50,7 +50,6 @@ import org.apache.lucene.util.automaton.Automata;
 import org.apache.lucene.util.automaton.Automaton;
 import org.apache.lucene.util.automaton.Operations;
 import org.apache.lucene.util.automaton.TooComplexToDeterminizeException;
-import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.compress.CompressedXContent;
@@ -58,7 +57,6 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.Fuzziness;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
-import org.elasticsearch.index.mapper.FieldNamesFieldMapper;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.search.QueryStringQueryParser;
 import org.elasticsearch.search.internal.SearchContext;
@@ -1053,36 +1051,6 @@ public class QueryStringQueryBuilderTests extends AbstractQueryTestCase<QueryStr
         assertThat(query, equalTo(expected));
     }
 
-    public void testDisabledFieldNamesField() throws Exception {
-        QueryShardContext context = createShardContext();
-        context.getMapperService().merge("_doc",
-                new CompressedXContent(Strings
-                        .toString(PutMappingRequest.buildFromSimplifiedDef("_doc",
-                                "foo",
-                                "type=text",
-                                "_field_names",
-                                "enabled=false"))),
-                MapperService.MergeReason.MAPPING_UPDATE);
-
-        try {
-            QueryStringQueryBuilder queryBuilder = new QueryStringQueryBuilder("foo:*");
-            Query query = queryBuilder.toQuery(context);
-            Query expected = new WildcardQuery(new Term("foo", "*"));
-            assertThat(query, equalTo(expected));
-        } finally {
-            // restore mappings as they were before
-            context.getMapperService().merge("_doc",
-                    new CompressedXContent(Strings.toString(
-                            PutMappingRequest.buildFromSimplifiedDef("_doc",
-                                    "foo",
-                                    "type=text",
-                                    "_field_names",
-                                    "enabled=true"))),
-                    MapperService.MergeReason.MAPPING_UPDATE);
-        }
-        assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}",  context.index().getName()));
-    }
-
     public void testFromJson() throws IOException {
         String json =
                 "{\n" +