Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate _field_names disabling #42854

Merged
merged 10 commits into from
Sep 11, 2019
Merged

Deprecate _field_names disabling #42854

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9635484
Deprecate and ignore `_field_names` disabling
Jun 4, 2019
26e8b64
iter
Jun 5, 2019
e4620c6
Merge branch 'master' into fix-27239
Aug 5, 2019
d46bf47
Only deprecate and log
Aug 5, 2019
c2e5503
Merge branch 'master' into fix-27239
Aug 9, 2019
4308e8e
Log deprecation on any index version, inlcude name in log
Aug 9, 2019
b653c5d
iter
Aug 9, 2019
96ef8f8
Merge branch 'master' into fix-27239
Aug 9, 2019
c4a0cb0
Merge branch 'master' into fix-27239
Sep 9, 2019
c552702
Changes to deprecation message and revert removing test
Sep 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/reference/mapping/fields/field-names-field.asciidoc
View file
Open in desktop
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 `enabled` setting for `_field_names`, which is no longer necessary. If possible, remove it from your mappings and templates. The setting will be removed in a future major version.]
11 changes: 4 additions & 7 deletions ...les/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java
View file
Open in desktop
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 @@ -590,7 +589,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 @@ -605,7 +603,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 @@ -617,7 +615,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 @@ -635,7 +632,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 @@ -646,7 +643,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
7 changes: 7 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ public FieldNamesFieldMapper build(BuilderContext context) {
}

public static class TypeParser implements MetadataFieldMapper.TypeParser {

public static final String ENABLED_DEPRECATION_MESSAGE = "Index [{}] uses the `enabled` setting for `_field_names`, which is no "
cbuescher marked this conversation as resolved.
Show resolved Hide resolved
+ "longer necessary. If possible, remove it from your mappings and templates. The setting will be removed in a future "
+ "major version.";

@Override
public MetadataFieldMapper.Builder<?,?> parse(String name, Map<String, Object> node,
ParserContext parserContext) throws MapperParsingException {
Expand All @@ -105,6 +110,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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should still support this field (and log the deprecation) for indices created in a version where the option is available (7x) and throw an error if the version is on or after 8.0 ?

}
Expand Down
21 changes: 2 additions & 19 deletions server/src/test/java/org/elasticsearch/index/mapper/FieldNamesFieldMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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,24 +134,6 @@ public void testDisabled() throws Exception {
XContentType.JSON));

assertNull(doc.rootDoc().get("_field_names"));
}

public void testMergingMappings() throws Exception {
cbuescher marked this conversation as resolved.
Show resolved Hide resolved
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();

DocumentMapper mapperEnabled = mapperService.merge("type", new CompressedXContent(enabledMapping),
MapperService.MergeReason.MAPPING_UPDATE);
DocumentMapper mapperDisabled = mapperService.merge("type", new CompressedXContent(disabledMapping),
MapperService.MergeReason.MAPPING_UPDATE);
assertFalse(mapperDisabled.metadataMapper(FieldNamesFieldMapper.class).fieldType().isEnabled());

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"));
}
}
29 changes: 17 additions & 12 deletions server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,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 @@ -1055,11 +1056,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 @@ -1068,16 +1072,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