Browse Source

Deprecate `_field_names` disabling (#42854)

Currently we allow `_field_names` fields to be disabled explicitely, but since
the overhead is negligible now we decided to keep it turned on by default and
deprecate the `enable` option on the field type. This change adds a deprecation
warning whenever this setting is used, going forward we want to ignore and finally
remove it.

Closes #27239
Christoph Büscher 6 years ago
parent
commit
d0a7bbcb69

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

@@ -14,7 +14,9 @@ be available but will not use the `_field_names` field.
 [[disable-field-names]]
 ==== Disabling `_field_names`
 
-Disabling `_field_names` is often not necessary because it no longer
+NOTE: Disabling `_field_names` has been deprecated and will be removed in a future major version.
+
+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
@@ -31,3 +33,5 @@ PUT tweets
   }
 }
 --------------------------------------------------
+// CONSOLE
+// 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.]

+ 4 - 7
modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java

@@ -146,7 +146,6 @@ public class PercolatorFieldMapperTests extends ESSingleNodeTestCase {
         mapperService = indexService.mapperService();
 
         String mapper = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("doc")
-            .startObject("_field_names").field("enabled", false).endObject() // makes testing easier
             .startObject("properties")
                 .startObject("field").field("type", "text").endObject()
                 .startObject("field1").field("type", "text").endObject()
@@ -322,7 +321,7 @@ public class PercolatorFieldMapperTests extends ESSingleNodeTestCase {
         ParseContext.Document document = parseContext.doc();
 
         PercolatorFieldMapper.FieldType fieldType = (PercolatorFieldMapper.FieldType) fieldMapper.fieldType();
-        assertThat(document.getFields().size(), equalTo(3));
+        assertThat(document.getFields().size(), equalTo(4));
         assertThat(document.getFields().get(0).binaryValue().utf8ToString(), equalTo("field\u0000term"));
         assertThat(document.getField(fieldType.extractionResultField.name()).stringValue(), equalTo(EXTRACTION_PARTIAL));
     }
@@ -590,7 +589,6 @@ public class PercolatorFieldMapperTests extends ESSingleNodeTestCase {
     public void testMultiplePercolatorFields() throws Exception {
         String typeName = "doc";
         String percolatorMapper = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(typeName)
-                .startObject("_field_names").field("enabled", false).endObject() // makes testing easier
                 .startObject("properties")
                     .startObject("query_field1").field("type", "percolator").endObject()
                     .startObject("query_field2").field("type", "percolator").endObject()
@@ -605,7 +603,7 @@ public class PercolatorFieldMapperTests extends ESSingleNodeTestCase {
                         .field("query_field2", queryBuilder)
                         .endObject()),
                         XContentType.JSON));
-        assertThat(doc.rootDoc().getFields().size(), equalTo(14)); // also includes all other meta fields
+        assertThat(doc.rootDoc().getFields().size(), equalTo(16)); // also includes all other meta fields
         BytesRef queryBuilderAsBytes = doc.rootDoc().getField("query_field1.query_builder_field").binaryValue();
         assertQueryBuilder(queryBuilderAsBytes, queryBuilder);
 
@@ -617,7 +615,6 @@ public class PercolatorFieldMapperTests extends ESSingleNodeTestCase {
     public void testNestedPercolatorField() throws Exception {
         String typeName = "doc";
         String percolatorMapper = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject(typeName)
-                .startObject("_field_names").field("enabled", false).endObject() // makes testing easier
                 .startObject("properties")
                 .startObject("object_field")
                     .field("type", "object")
@@ -635,7 +632,7 @@ public class PercolatorFieldMapperTests extends ESSingleNodeTestCase {
                             .field("query_field", queryBuilder)
                         .endObject().endObject()),
                         XContentType.JSON));
-        assertThat(doc.rootDoc().getFields().size(), equalTo(10)); // also includes all other meta fields
+        assertThat(doc.rootDoc().getFields().size(), equalTo(12)); // also includes all other meta fields
         BytesRef queryBuilderAsBytes = doc.rootDoc().getField("object_field.query_field.query_builder_field").binaryValue();
         assertQueryBuilder(queryBuilderAsBytes, queryBuilder);
 
@@ -646,7 +643,7 @@ public class PercolatorFieldMapperTests extends ESSingleNodeTestCase {
                             .endArray()
                         .endObject()),
                         XContentType.JSON));
-        assertThat(doc.rootDoc().getFields().size(), equalTo(10)); // also includes all other meta fields
+        assertThat(doc.rootDoc().getFields().size(), equalTo(12)); // also includes all other meta fields
         queryBuilderAsBytes = doc.rootDoc().getField("object_field.query_field.query_builder_field").binaryValue();
         assertQueryBuilder(queryBuilderAsBytes, queryBuilder);
 

+ 7 - 0
server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java

@@ -95,6 +95,11 @@ public class FieldNamesFieldMapper extends MetadataFieldMapper {
     }
 
     public static class TypeParser implements MetadataFieldMapper.TypeParser {
+
+        public static final String ENABLED_DEPRECATION_MESSAGE = "Index [{}] 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.";
+
         @Override
         public MetadataFieldMapper.Builder<?,?> parse(String name, Map<String, Object> node,
                                                       ParserContext parserContext) throws MapperParsingException {
@@ -105,6 +110,8 @@ public class FieldNamesFieldMapper extends MetadataFieldMapper {
                 String fieldName = entry.getKey();
                 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"));
                     iterator.remove();
                 }

+ 4 - 1
server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java

@@ -43,7 +43,7 @@ public class FieldNamesFieldMapperTests extends ESSingleNodeTestCase {
         return set;
     }
 
-    private static <T> SortedSet<T> set(T... values) {
+    private static SortedSet<String> set(String... values) {
         return new TreeSet<>(Arrays.asList(values));
     }
 
@@ -114,6 +114,7 @@ public class FieldNamesFieldMapperTests extends ESSingleNodeTestCase {
             XContentType.JSON));
 
         assertFieldNames(set("field"), doc);
+        assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test"));
     }
 
     public void testDisabled() throws Exception {
@@ -133,6 +134,7 @@ public class FieldNamesFieldMapperTests extends ESSingleNodeTestCase {
             XContentType.JSON));
 
         assertNull(doc.rootDoc().get("_field_names"));
+        assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test"));
     }
 
     public void testMergingMappings() throws Exception {
@@ -152,5 +154,6 @@ public class FieldNamesFieldMapperTests extends ESSingleNodeTestCase {
 
         mapperEnabled = mapperService.merge("type", new CompressedXContent(enabledMapping), MapperService.MergeReason.MAPPING_UPDATE);
         assertTrue(mapperEnabled.metadataMapper(FieldNamesFieldMapper.class).fieldType().isEnabled());
+        assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test"));
     }
 }

+ 17 - 12
server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java

@@ -58,6 +58,7 @@ 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;
@@ -1055,11 +1056,14 @@ public class QueryStringQueryBuilderTests extends AbstractQueryTestCase<QueryStr
     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);
+                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);
@@ -1068,16 +1072,17 @@ public class QueryStringQueryBuilderTests extends AbstractQueryTestCase<QueryStr
         } 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);
+                    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" +