From 6823e3cfdf464b687cc41efdfc0a1f5ce4eeca93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Thu, 12 Sep 2019 16:39:09 +0200 Subject: [PATCH 1/4] Disallow `_field_names` enabled setting After deprecating the setting starting with 7.5 in #42854, this change disallows setting enabled for the `_field_names` field on new indices. The setting continues to work for indices created in 7.x where we still emit a deprecation warning. --- .../mapping/fields/field-names-field.asciidoc | 24 ++------- .../migration/migrate_8_0/mappings.asciidoc | 10 +++- .../index/mapper/FieldNamesFieldMapper.java | 13 +++-- .../mapper/FieldNamesFieldMapperTests.java | 54 ++++++++++++------- .../query/QueryStringQueryBuilderTests.java | 32 ----------- 5 files changed, 58 insertions(+), 75 deletions(-) diff --git a/docs/reference/mapping/fields/field-names-field.asciidoc b/docs/reference/mapping/fields/field-names-field.asciidoc index cec9d8b0d37c3..13ac7ff7eafcb 100644 --- a/docs/reference/mapping/fields/field-names-field.asciidoc +++ b/docs/reference/mapping/fields/field-names-field.asciidoc @@ -14,24 +14,8 @@ 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 usually no longer possible. It is 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 - } - } -} --------------------------------------------------- -// 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.] +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. diff --git a/docs/reference/migration/migrate_8_0/mappings.asciidoc b/docs/reference/migration/migrate_8_0/mappings.asciidoc index 16e75473885c6..8ee589cda1269 100644 --- a/docs/reference/migration/migrate_8_0/mappings.asciidoc +++ b/docs/reference/migration/migrate_8_0/mappings.asciidoc @@ -22,4 +22,12 @@ 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. \ No newline at end of file +blocks into a single level, or by switching to `copy_to` if appropriate. + +[float] +==== 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. diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java index 84211e69cf420..287ec5e54d72f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java @@ -24,6 +24,7 @@ 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 @@ */ 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 MetadataFieldMapper.Builder parse(String name, Map n 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` 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(); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java index a1ad67a0550ae..006ae75ab935b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java +++ b/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 void testInjectIntoDocDuringParsing() throws Exception { 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` has been deprecated and removed butis 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 void testDisabled() throws Exception { 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 void testMergingMappings() throws Exception { 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; + } } diff --git a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java index c1d1869dbbcbf..7ccb3dffa6228 100644 --- a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -50,7 +50,6 @@ 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.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 void testExistsFieldQuery() throws Exception { 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" + From f116044a9f53fca7c1adba92e5892036777f9fbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 20 Sep 2019 10:46:23 +0200 Subject: [PATCH 2/4] Updating error message --- .../org/elasticsearch/index/mapper/FieldNamesFieldMapper.java | 4 ++-- .../index/mapper/FieldNamesFieldMapperTests.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java index 287ec5e54d72f..ae16aab41686d 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java @@ -112,8 +112,8 @@ public MetadataFieldMapper.Builder parse(String name, Map n if (fieldName.equals("enabled")) { String indexName = parserContext.mapperService().index().getName(); if (parserContext.indexVersionCreated().onOrAfter(Version.V_8_0_0)) { - throw new MapperParsingException("The `enabled` setting for the `_field_names` has been deprecated and removed but" - + "is still used in index [{}]. Please remove it from your mappings and templates."); + 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")); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java index 006ae75ab935b..43ba11e4ffe6b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java @@ -108,7 +108,7 @@ public void testUsingEnabledSettingThrows() throws Exception { MapperParsingException ex = expectThrows(MapperParsingException.class, () -> createIndex("test").mapperService().documentMapperParser() .parse("type", new CompressedXContent(mapping))); - assertEquals("The `enabled` setting for the `_field_names` has been deprecated and removed butis still used in index [{}]. " + 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()); } From 2ac58836c202175380023d25a8fe9484c67753f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Fri, 20 Sep 2019 11:44:33 +0200 Subject: [PATCH 3/4] Line length --- .../index/mapper/FieldNamesFieldMapperTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java index 43ba11e4ffe6b..639274c0c6a19 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java @@ -105,8 +105,8 @@ public void testUsingEnabledSettingThrows() throws Exception { .startObject("properties") .startObject("field").field("type", "keyword").field("doc_values", false).endObject() .endObject().endObject().endObject()); - MapperParsingException ex = expectThrows(MapperParsingException.class, () -> createIndex("test").mapperService().documentMapperParser() - .parse("type", new CompressedXContent(mapping))); + MapperParsingException ex = expectThrows(MapperParsingException.class, + () -> createIndex("test").mapperService().documentMapperParser().parse("type", new CompressedXContent(mapping))); 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()); From d281a3459237ff6b4079d5a27d5b2a1f55ce7562 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Mon, 23 Sep 2019 12:46:56 +0200 Subject: [PATCH 4/4] Add migration notes anchor --- docs/reference/migration/migrate_8_0/mappings.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/reference/migration/migrate_8_0/mappings.asciidoc b/docs/reference/migration/migrate_8_0/mappings.asciidoc index 8ee589cda1269..2846c236532c6 100644 --- a/docs/reference/migration/migrate_8_0/mappings.asciidoc +++ b/docs/reference/migration/migrate_8_0/mappings.asciidoc @@ -25,6 +25,7 @@ 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. [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.