Skip to content

Remove support for _type in searches #68564

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.

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

Merged
merged 3 commits into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,7 @@ public void testUnsupportedQueries() {
PercolatorFieldMapper.verifyQuery(rangeQuery1);
PercolatorFieldMapper.verifyQuery(rangeQuery2);

HasChildQueryBuilder hasChildQuery = new HasChildQueryBuilder("_type", new MatchAllQueryBuilder(), ScoreMode.None);
HasChildQueryBuilder hasChildQuery = new HasChildQueryBuilder("parent", new MatchAllQueryBuilder(), ScoreMode.None);
expectThrows(IllegalArgumentException.class, () ->
PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(hasChildQuery)));
expectThrows(IllegalArgumentException.class, () ->
Expand All @@ -671,7 +671,7 @@ public void testUnsupportedQueries() {
expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(hasChildQuery));
expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(hasChildQuery)));

HasParentQueryBuilder hasParentQuery = new HasParentQueryBuilder("_type", new MatchAllQueryBuilder(), false);
HasParentQueryBuilder hasParentQuery = new HasParentQueryBuilder("parent", new MatchAllQueryBuilder(), false);
expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(hasParentQuery));
expectThrows(IllegalArgumentException.class, () -> PercolatorFieldMapper.verifyQuery(new BoolQueryBuilder().must(hasParentQuery)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -951,8 +951,8 @@ public void testIndexOrDocValuesQuery() {

public void testToParentBlockJoinQuery() {
TermQuery termQuery = new TermQuery(new Term("field", "value"));
QueryBitSetProducer queryBitSetProducer = new QueryBitSetProducer(new TermQuery(new Term("_type", "child")));
ESToParentBlockJoinQuery query = new ESToParentBlockJoinQuery(termQuery, queryBitSetProducer, ScoreMode.None, "child");
QueryBitSetProducer queryBitSetProducer = new QueryBitSetProducer(new TermQuery(new Term("_nested_path", "nested")));
ESToParentBlockJoinQuery query = new ESToParentBlockJoinQuery(termQuery, queryBitSetProducer, ScoreMode.None, "nested");
Result result = analyze(query);
assertFalse(result.verified);
assertThat(result.minimumShouldMatch, equalTo(1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -562,22 +562,6 @@ setup:

- match: {hits.total: 4}

---
Copy link
Contributor

Choose a reason for hiding this comment

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

is this the only yaml test case that will be affected? Just wanted to make sure we have coverage in 7.x to bring this with compatible api

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this is the only one, we had very light test coverage for this capability.

"Test exists query on _type field":

- do:
allowed_warnings:
- "[types removal] Using the _type field in queries and aggregations is deprecated, prefer to use a field instead."
search:
rest_total_hits_as_int: true
index: test
body:
query:
exists:
field: _type

- match: {hits.total: 4}

---
"Test exists query on _routing field":
- do:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import static org.elasticsearch.client.Requests.getRequest;
import static org.elasticsearch.client.Requests.indexRequest;
import static org.elasticsearch.client.Requests.refreshRequest;
import static org.elasticsearch.index.query.QueryBuilders.termQuery;
import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
Expand Down Expand Up @@ -149,7 +149,7 @@ public void testIndexActions() throws Exception {
// check count
for (int i = 0; i < 5; i++) {
// test successful
SearchResponse countResponse = client().prepareSearch("test").setSize(0).setQuery(termQuery("_type", "_doc"))
SearchResponse countResponse = client().prepareSearch("test").setSize(0).setQuery(matchAllQuery())
.execute().actionGet();
assertNoFailures(countResponse);
assertThat(countResponse.getHits().getTotalHits().value, equalTo(2L));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1224,7 +1224,7 @@ public void testWithRescore() {
.addAggregation(terms("terms")
.field(TERMS_AGGS_FIELD)
.subAggregation(
topHits("hits").sort(SortBuilders.fieldSort("_type"))
topHits("hits").sort(SortBuilders.fieldSort("_index"))
)
)
.get();
Expand All @@ -1246,7 +1246,7 @@ public void testWithRescore() {
.addAggregation(terms("terms")
.field(TERMS_AGGS_FIELD)
.subAggregation(
topHits("hits").sort(SortBuilders.scoreSort()).sort(SortBuilders.fieldSort("_type"))
topHits("hits").sort(SortBuilders.scoreSort()).sort(SortBuilders.fieldSort("_index"))
)
)
.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,13 @@ final class FieldTypeLookup {
* For convenience, the set of copied fields includes the field itself.
*/
private final Map<String, Set<String>> fieldToCopiedFields = new HashMap<>();
private final String type;
private final DynamicKeyFieldTypeLookup dynamicKeyLookup;

FieldTypeLookup(
String type,
Collection<FieldMapper> fieldMappers,
Collection<FieldAliasMapper> fieldAliasMappers,
Collection<RuntimeFieldType> runtimeFieldTypes
) {
this.type = type;
Map<String, DynamicKeyFieldMapper> dynamicKeyMappers = new HashMap<>();

for (FieldMapper fieldMapper : fieldMappers) {
Expand Down Expand Up @@ -84,10 +81,6 @@ final class FieldTypeLookup {
* Returns the mapped field type for the given field name.
*/
MappedFieldType get(String field) {
if (field.equals(TypeFieldType.NAME)) {
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 this means we can remove the 'type' field and constructor parameter as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, yep I'll remove it.

return new TypeFieldType(type);
}

MappedFieldType fieldType = fullNameToFieldType.get(field);
if (fieldType != null) {
return fieldType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public MappingLookup(Mapping mapping,
}
}

this.fieldTypeLookup = new FieldTypeLookup(mapping.root().name(), mappers, aliasMappers, mapping.root().runtimeFieldTypes());
this.fieldTypeLookup = new FieldTypeLookup(mappers, aliasMappers, mapping.root().runtimeFieldTypes());
this.fieldMappers = Collections.unmodifiableMap(fieldMappers);
this.objectMappers = Collections.unmodifiableMap(objects);
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ public void testSegmentsWithMergeFlag() throws Exception {
}

public void testSegmentsWithIndexSort() throws Exception {
Sort indexSort = new Sort(new SortedSetSortField("_type", false));
Sort indexSort = new Sort(new SortedSetSortField("field", false));
try (Store store = createStore();
Engine engine =
createEngine(defaultSettings, store, createTempDir(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
public class FieldTypeLookupTests extends ESTestCase {

public void testEmpty() {
FieldTypeLookup lookup = new FieldTypeLookup("_doc", Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
FieldTypeLookup lookup = new FieldTypeLookup(Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
assertNull(lookup.get("foo"));
Collection<String> names = lookup.simpleMatchToFullName("foo");
assertNotNull(names);
Expand All @@ -37,7 +37,7 @@ public void testFilter() {
Collection<FieldAliasMapper> fieldAliases = singletonList(new FieldAliasMapper("alias", "alias", "test"));
Collection<RuntimeFieldType> runtimeFields = List.of(
new TestRuntimeField("runtime", "type"), new TestRuntimeField("field", "type"));
FieldTypeLookup fieldTypeLookup = new FieldTypeLookup("_doc", fieldMappers, fieldAliases, runtimeFields);
FieldTypeLookup fieldTypeLookup = new FieldTypeLookup(fieldMappers, fieldAliases, runtimeFields);
assertEquals(3, size(fieldTypeLookup.filter(ft -> true)));
for (MappedFieldType fieldType : fieldTypeLookup.filter(ft -> true)) {
if (fieldType.name().equals("test")) {
Expand Down Expand Up @@ -66,7 +66,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(Collections.singletonList(f), emptyList(), Collections.emptyList());
assertNull(lookup.get("bar"));
assertEquals(f.fieldType(), lookup.get("foo"));
assertEquals(1, size(lookup.filter(ft -> true)));
Expand All @@ -76,7 +76,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(Collections.singletonList(field), Collections.singletonList(alias),
Collections.emptyList());

MappedFieldType aliasType = lookup.get("alias");
Expand All @@ -90,7 +90,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(List.of(field1, field2), List.of(alias1, alias2), List.of());

Collection<String> names = lookup.simpleMatchToFullName("b*");

Expand All @@ -107,7 +107,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(singletonList(field), emptyList(), emptyList());

assertEquals(Set.of("field"), lookup.sourcePaths("field"));
assertEquals(Set.of("field"), lookup.sourcePaths("field.subfield1"));
Expand All @@ -123,25 +123,17 @@ public void testSourcePathsWithCopyTo() {
.copyTo("field")
.build(new ContentPath());

FieldTypeLookup lookup = new FieldTypeLookup("_doc", Arrays.asList(field, otherField), emptyList(), emptyList());
FieldTypeLookup lookup = new FieldTypeLookup(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"));
}

public void testTypeLookup() {
String type = randomAlphaOfLength(4);
assertThat(
((TypeFieldType) new FieldTypeLookup(type, List.of(), List.of(), List.of()).get(TypeFieldType.NAME)).getType(),
equalTo(type)
);
}

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(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)));
Expand All @@ -155,7 +147,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(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));
Expand All @@ -170,7 +162,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(List.of(field1, concrete), emptyList(), List.of(field2, subfield));
{
Set<String> matches = fieldTypeLookup.simpleMatchToFullName("fie*");
assertEquals(2, matches.size());
Expand All @@ -192,7 +184,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(List.of(field1, concrete), emptyList(), List.of(field2, subfield));
{
Set<String> sourcePaths = fieldTypeLookup.sourcePaths("field1");
assertEquals(1, sourcePaths.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ public void testReorderParent() throws IOException {

assertThat(doc.docs().size(), equalTo(3));
if (version.before(Version.V_8_0_0)) {
assertThat(doc.docs().get(0).get(TypeFieldType.NAME), equalTo(nested1Mapper.nestedTypePath()));
assertThat(doc.docs().get(0).get("_type"), equalTo(nested1Mapper.nestedTypePath()));
} else {
assertThat(doc.docs().get(0).get(NestedPathFieldMapper.NAME), equalTo(nested1Mapper.nestedTypePath()));
}
Expand Down
Loading