From d03516fe8c001ab8a0ffada7b838b5156ae0fefc Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 2 Feb 2021 11:32:33 -0800 Subject: [PATCH] Fix error when requesting _type from fields option. For 8.0 indices, the 'fields' option would throw an error when '_type' is requested. Since '_type' was removed in 8.0, it should instead behave as if the field didn't exist, where we don't error but return no values. --- .../index/mapper/FieldTypeLookup.java | 14 ++++++++--- .../index/mapper/MappingLookup.java | 8 +++++- .../index/mapper/FieldTypeLookupTests.java | 25 ++++++++++--------- .../fetch/subphase/FieldFetcherTests.java | 14 ++++++++++- .../mapper/FlattenedFieldLookupTests.java | 10 ++++---- 5 files changed, 48 insertions(+), 23 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java index 22a904daf12b5..63b8dc025e77f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldTypeLookup.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.mapper; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.regex.Regex; import java.util.Collection; @@ -44,16 +45,21 @@ final class FieldTypeLookup { * For convenience, the set of copied fields includes the field itself. */ private final Map> fieldToCopiedFields = new HashMap<>(); - private final String type; private final DynamicKeyFieldTypeLookup dynamicKeyLookup; + /** + * A field type representing the document type. This will be null for 8.0 indices, + * which don't have a type and do not support using the _type field in searches. + */ + @Nullable private final TypeFieldType typeFieldType; + FieldTypeLookup( - String type, + @Nullable TypeFieldType typeFieldType, Collection fieldMappers, Collection fieldAliasMappers, Collection runtimeFieldTypes ) { - this.type = type; + this.typeFieldType = typeFieldType; Map dynamicKeyMappers = new HashMap<>(); for (FieldMapper fieldMapper : fieldMappers) { @@ -96,7 +102,7 @@ final class FieldTypeLookup { */ MappedFieldType get(String field) { if (field.equals(TypeFieldType.NAME)) { - return new TypeFieldType(type); + return typeFieldType; } MappedFieldType fieldType = fullNameToFieldType.get(field); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java index 09e264f046094..70ce857408abf 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MappingLookup.java @@ -19,6 +19,7 @@ package org.elasticsearch.index.mapper; +import org.elasticsearch.Version; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.analysis.IndexAnalyzers; import org.elasticsearch.index.analysis.NamedAnalyzer; @@ -159,7 +160,12 @@ public MappingLookup(Mapping mapping, } } - this.fieldTypeLookup = new FieldTypeLookup(mapping.root().name(), mappers, aliasMappers, mapping.root().runtimeFieldTypes()); + TypeFieldType typeFieldType = null; + if (mapping != Mapping.EMPTY && indexSettings.getIndexVersionCreated().before(Version.V_8_0_0)) { + typeFieldType = new TypeFieldType(mapping.root().typeName()); + } + + this.fieldTypeLookup = new FieldTypeLookup(typeFieldType, mappers, aliasMappers, mapping.root().runtimeFieldTypes()); this.fieldMappers = Collections.unmodifiableMap(fieldMappers); this.objectMappers = Collections.unmodifiableMap(objects); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java index 7f4e05f3a398b..2de011751a80c 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldTypeLookupTests.java @@ -35,7 +35,7 @@ public class FieldTypeLookupTests extends ESTestCase { public void testEmpty() { - FieldTypeLookup lookup = new FieldTypeLookup("_doc", Collections.emptyList(), Collections.emptyList(), Collections.emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(null, Collections.emptyList(), Collections.emptyList(), Collections.emptyList()); assertNull(lookup.get("foo")); Collection names = lookup.simpleMatchToFullName("foo"); assertNotNull(names); @@ -48,7 +48,7 @@ public void testFilter() { Collection fieldAliases = singletonList(new FieldAliasMapper("alias", "alias", "test")); Collection runtimeFields = List.of( new TestRuntimeField("runtime", "type"), new TestRuntimeField("field", "type")); - FieldTypeLookup fieldTypeLookup = new FieldTypeLookup("_doc", fieldMappers, fieldAliases, runtimeFields); + FieldTypeLookup fieldTypeLookup = new FieldTypeLookup(null, fieldMappers, fieldAliases, runtimeFields); assertEquals(3, size(fieldTypeLookup.filter(ft -> true))); for (MappedFieldType fieldType : fieldTypeLookup.filter(ft -> true)) { if (fieldType.name().equals("test")) { @@ -77,7 +77,7 @@ public void testFilter() { public void testAddNewField() { MockFieldMapper f = new MockFieldMapper("foo"); - FieldTypeLookup lookup = new FieldTypeLookup("_doc", Collections.singletonList(f), emptyList(), Collections.emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(null, Collections.singletonList(f), emptyList(), Collections.emptyList()); assertNull(lookup.get("bar")); assertEquals(f.fieldType(), lookup.get("foo")); assertEquals(1, size(lookup.filter(ft -> true))); @@ -87,7 +87,7 @@ public void testAddFieldAlias() { MockFieldMapper field = new MockFieldMapper("foo"); FieldAliasMapper alias = new FieldAliasMapper("alias", "alias", "foo"); - FieldTypeLookup lookup = new FieldTypeLookup("_doc", Collections.singletonList(field), Collections.singletonList(alias), + FieldTypeLookup lookup = new FieldTypeLookup(null, Collections.singletonList(field), Collections.singletonList(alias), Collections.emptyList()); MappedFieldType aliasType = lookup.get("alias"); @@ -101,7 +101,7 @@ public void testSimpleMatchToFullName() { FieldAliasMapper alias1 = new FieldAliasMapper("food", "food", "foo"); FieldAliasMapper alias2 = new FieldAliasMapper("barometer", "barometer", "bar"); - FieldTypeLookup lookup = new FieldTypeLookup("_doc", List.of(field1, field2), List.of(alias1, alias2), List.of()); + FieldTypeLookup lookup = new FieldTypeLookup(null, List.of(field1, field2), List.of(alias1, alias2), List.of()); Collection names = lookup.simpleMatchToFullName("b*"); @@ -118,7 +118,7 @@ public void testSourcePathWithMultiFields() { .addMultiField(new MockFieldMapper.Builder("field.subfield2")) .build(new ContentPath()); - FieldTypeLookup lookup = new FieldTypeLookup("_doc", singletonList(field), emptyList(), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(null, singletonList(field), emptyList(), emptyList()); assertEquals(Set.of("field"), lookup.sourcePaths("field")); assertEquals(Set.of("field"), lookup.sourcePaths("field.subfield1")); @@ -134,7 +134,7 @@ public void testSourcePathsWithCopyTo() { .copyTo("field") .build(new ContentPath()); - FieldTypeLookup lookup = new FieldTypeLookup("_doc", Arrays.asList(field, otherField), emptyList(), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(null, Arrays.asList(field, otherField), emptyList(), emptyList()); assertEquals(Set.of("other_field", "field"), lookup.sourcePaths("field")); assertEquals(Set.of("other_field", "field"), lookup.sourcePaths("field.subfield1")); @@ -142,8 +142,9 @@ public void testSourcePathsWithCopyTo() { public void testTypeLookup() { String type = randomAlphaOfLength(4); + TypeFieldType fieldType = new TypeFieldType(type); assertThat( - ((TypeFieldType) new FieldTypeLookup(type, List.of(), List.of(), List.of()).get(TypeFieldType.NAME)).getType(), + ((TypeFieldType) new FieldTypeLookup(fieldType, List.of(), List.of(), List.of()).get(TypeFieldType.NAME)).getType(), equalTo(type) ); } @@ -152,7 +153,7 @@ public void testRuntimeFieldsLookup() { MockFieldMapper concrete = new MockFieldMapper("concrete"); TestRuntimeField runtime = new TestRuntimeField("runtime", "type"); - FieldTypeLookup fieldTypeLookup = new FieldTypeLookup("_doc", List.of(concrete), emptyList(), List.of(runtime)); + FieldTypeLookup fieldTypeLookup = new FieldTypeLookup(null, List.of(concrete), emptyList(), List.of(runtime)); assertThat(fieldTypeLookup.get("concrete"), instanceOf(MockFieldMapper.FakeFieldType.class)); assertThat(fieldTypeLookup.get("runtime"), instanceOf(TestRuntimeField.class)); assertEquals(2, size(fieldTypeLookup.filter(ft -> true))); @@ -166,7 +167,7 @@ public void testRuntimeFieldOverrides() { TestRuntimeField subfieldOverride = new TestRuntimeField("object.subfield", "type"); TestRuntimeField runtime = new TestRuntimeField("runtime", "type"); - FieldTypeLookup fieldTypeLookup = new FieldTypeLookup("_doc", List.of(field, concrete, subfield), emptyList(), + FieldTypeLookup fieldTypeLookup = new FieldTypeLookup(null, List.of(field, concrete, subfield), emptyList(), List.of(fieldOverride, runtime, subfieldOverride)); assertThat(fieldTypeLookup.get("field"), instanceOf(TestRuntimeField.class)); assertThat(fieldTypeLookup.get("object.subfield"), instanceOf(TestRuntimeField.class)); @@ -181,7 +182,7 @@ public void testRuntimeFieldsSimpleMatchToFullName() { TestRuntimeField field2 = new TestRuntimeField("field2", "type"); TestRuntimeField subfield = new TestRuntimeField("object.subfield", "type"); - FieldTypeLookup fieldTypeLookup = new FieldTypeLookup("_doc", List.of(field1, concrete), emptyList(), List.of(field2, subfield)); + FieldTypeLookup fieldTypeLookup = new FieldTypeLookup(null, List.of(field1, concrete), emptyList(), List.of(field2, subfield)); { Set matches = fieldTypeLookup.simpleMatchToFullName("fie*"); assertEquals(2, matches.size()); @@ -203,7 +204,7 @@ public void testRuntimeFieldsSourcePaths() { TestRuntimeField field2 = new TestRuntimeField("field2", "type"); TestRuntimeField subfield = new TestRuntimeField("object.subfield", "type"); - FieldTypeLookup fieldTypeLookup = new FieldTypeLookup("_doc", List.of(field1, concrete), emptyList(), List.of(field2, subfield)); + FieldTypeLookup fieldTypeLookup = new FieldTypeLookup(null, List.of(field1, concrete), emptyList(), List.of(field2, subfield)); { Set sourcePaths = fieldTypeLookup.sourcePaths("field1"); assertEquals(1, sourcePaths.size()); diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java index 60bc6dcd06a96..651e82a29b828 100644 --- a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java +++ b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java @@ -118,7 +118,7 @@ public void testNonExistentField() throws IOException { .endObject(); Map fields = fetchFields(mapperService, source, "non-existent"); - assertThat(fields.size(), equalTo(0)); + assertTrue(fields.isEmpty()); } public void testMetadataFields() throws IOException { @@ -131,6 +131,18 @@ public void testMetadataFields() throws IOException { assertTrue(fields.isEmpty()); } + public void testTypeMetadataField() throws IOException { + MapperService mapperService = createMapperService(); + XContentBuilder source = XContentFactory.jsonBuilder().startObject() + .field("field", "value") + .endObject(); + + // The _type field was deprecated in 7.x and is not supported in 8.0. So the behavior + // should be the same as if the field didn't exist. + Map fields = fetchFields(mapperService, source, "_type"); + assertTrue(fields.isEmpty()); + } + public void testFetchAllFields() throws IOException { MapperService mapperService = createMapperService(); XContentBuilder source = XContentFactory.jsonBuilder().startObject() diff --git a/x-pack/plugin/mapper-flattened/src/test/java/org/elasticsearch/index/mapper/FlattenedFieldLookupTests.java b/x-pack/plugin/mapper-flattened/src/test/java/org/elasticsearch/index/mapper/FlattenedFieldLookupTests.java index 681746532c83d..801dd371a72b8 100644 --- a/x-pack/plugin/mapper-flattened/src/test/java/org/elasticsearch/index/mapper/FlattenedFieldLookupTests.java +++ b/x-pack/plugin/mapper-flattened/src/test/java/org/elasticsearch/index/mapper/FlattenedFieldLookupTests.java @@ -39,7 +39,7 @@ public void testFieldTypeLookup() { String fieldName = "object1.object2.field"; FlattenedFieldMapper mapper = createFlattenedMapper(fieldName); - FieldTypeLookup lookup = new FieldTypeLookup("_doc", singletonList(mapper), emptyList(), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(null, singletonList(mapper), emptyList(), emptyList()); assertEquals(mapper.fieldType(), lookup.get(fieldName)); String objectKey = "key1.key2"; @@ -60,7 +60,7 @@ public void testFieldTypeLookupWithAlias() { String aliasName = "alias"; FieldAliasMapper alias = new FieldAliasMapper(aliasName, aliasName, fieldName); - FieldTypeLookup lookup = new FieldTypeLookup("_doc", singletonList(mapper), singletonList(alias), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(null, singletonList(mapper), singletonList(alias), emptyList()); assertEquals(mapper.fieldType(), lookup.get(aliasName)); String objectKey = "key1.key2"; @@ -83,11 +83,11 @@ public void testFieldTypeLookupWithMultipleFields() { FlattenedFieldMapper mapper2 = createFlattenedMapper(field2); FlattenedFieldMapper mapper3 = createFlattenedMapper(field3); - FieldTypeLookup lookup = new FieldTypeLookup("_doc", Arrays.asList(mapper1, mapper2), emptyList(), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(null, Arrays.asList(mapper1, mapper2), emptyList(), emptyList()); assertNotNull(lookup.get(field1 + ".some.key")); assertNotNull(lookup.get(field2 + ".some.key")); - lookup = new FieldTypeLookup("_doc", Arrays.asList(mapper1, mapper2, mapper3), emptyList(), emptyList()); + lookup = new FieldTypeLookup(null, Arrays.asList(mapper1, mapper2, mapper3), emptyList(), emptyList()); assertNotNull(lookup.get(field1 + ".some.key")); assertNotNull(lookup.get(field2 + ".some.key")); assertNotNull(lookup.get(field3 + ".some.key")); @@ -124,7 +124,7 @@ public void testFieldLookupIterator() { MockFieldMapper mapper = new MockFieldMapper("foo"); FlattenedFieldMapper flattenedMapper = createFlattenedMapper("object1.object2.field"); - FieldTypeLookup lookup = new FieldTypeLookup("_doc", Arrays.asList(mapper, flattenedMapper), emptyList(), emptyList()); + FieldTypeLookup lookup = new FieldTypeLookup(null, Arrays.asList(mapper, flattenedMapper), emptyList(), emptyList()); Set fieldNames = new HashSet<>(); lookup.filter(ft -> true).forEach(ft -> fieldNames.add(ft.name()));