Skip to content

Commit

Permalink
Deprecate _field_names disabling (#42854)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Christoph Büscher committed Sep 11, 2019
1 parent efea581 commit aa0c586
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 21 deletions.
6 changes: 5 additions & 1 deletion docs/reference/mapping/fields/field-names-field.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.]
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ public void init() throws Exception {
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()
Expand Down Expand Up @@ -322,7 +321,7 @@ public void testExtractTermsAndRanges_partial() throws Exception {
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));
}
Expand Down Expand Up @@ -610,7 +609,6 @@ public void testAllowNoAdditionalSettings() throws Exception {
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()
Expand All @@ -625,7 +623,7 @@ public void testMultiplePercolatorFields() throws Exception {
.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);

Expand All @@ -637,7 +635,6 @@ public void testMultiplePercolatorFields() throws Exception {
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")
Expand All @@ -655,7 +652,7 @@ public void testNestedPercolatorField() throws Exception {
.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);

Expand All @@ -666,7 +663,7 @@ public void testNestedPercolatorField() throws Exception {
.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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ public FieldNamesFieldMapper build(BuilderContext context) {
}

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 {
Expand All @@ -106,6 +111,8 @@ public MetadataFieldMapper.Builder<?,?> parse(String name, Map<String, Object> n
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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ private static SortedSet<String> extract(String path) {
return set;
}

private static <T> SortedSet<T> set(T... values) {
private static SortedSet<String> set(String... values) {
return new TreeSet<>(Arrays.asList(values));
}

Expand Down Expand Up @@ -114,6 +114,7 @@ public void testExplicitEnabled() throws Exception {
XContentType.JSON));

assertFieldNames(set("field"), doc);
assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test"));
}

public void testDisabled() throws Exception {
Expand All @@ -133,6 +134,7 @@ public void testDisabled() throws Exception {
XContentType.JSON));

assertNull(doc.rootDoc().get("_field_names"));
assertWarnings(FieldNamesFieldMapper.TypeParser.ENABLED_DEPRECATION_MESSAGE.replace("{}", "test"));
}

public void testMergingMappings() throws Exception {
Expand All @@ -152,5 +154,6 @@ public void testMergingMappings() throws Exception {

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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
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;
Expand Down Expand Up @@ -1058,11 +1059,14 @@ public void testExistsFieldQuery() throws Exception {
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);
Expand All @@ -1071,16 +1075,17 @@ public void testDisabledFieldNamesField() throws Exception {
} 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" +
Expand Down

0 comments on commit aa0c586

Please sign in to comment.