Skip to content

Deprecate uses of _type as a field name in queries #36503

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
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 @@ -823,10 +823,9 @@ public void testReindex() throws Exception {
// tag::reindex-request-conflicts
request.setConflicts("proceed"); // <1>
// end::reindex-request-conflicts
// tag::reindex-request-typeOrQuery
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should remove these reindex-related changes, and tackle them in a dedicated PR? I think there may be more updates to track down around the reindex 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.

@jtibshirani Thanks Julie. The only reason I added these changes into this PR is because request.setSourceDocTypes("_doc"); underneath uses a query with a type that we are deprecating in this PR, and if I don't remove this line, CRUDDocumentationIT. :: testReindex fails.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I missed that!

request.setSourceDocTypes("_doc"); // <1>
request.setSourceQuery(new TermQueryBuilder("user", "kimchy")); // <2>
// end::reindex-request-typeOrQuery
// tag::reindex-request-query
request.setSourceQuery(new TermQueryBuilder("user", "kimchy")); // <1>
// end::reindex-request-query
// tag::reindex-request-size
request.setSize(10); // <1>
// end::reindex-request-size
Expand Down
7 changes: 3 additions & 4 deletions docs/java-rest/high-level/document/reindex.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,13 @@ include-tagged::{doc-tests-file}[{api}-request-conflicts]
--------------------------------------------------
<1> Set `proceed` on version conflict

You can limit the documents by adding a type to the source or by adding a query.
You can limit the documents by adding a query.

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests-file}[{api}-request-typeOrQuery]
include-tagged::{doc-tests-file}[{api}-request-query]
--------------------------------------------------
<1> Only copy `doc` type
<2> Only copy documents which have field `user` set to `kimchy`
<1> Only copy documents which have field `user` set to `kimchy`

It’s also possible to limit the number of processed documents by setting size.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.index.mapper.TypeFieldMapper;
import org.elasticsearch.rest.action.document.RestGetAction;
import org.elasticsearch.rest.action.search.RestExplainAction;
import org.elasticsearch.cluster.metadata.IndexMetaData;
Expand Down Expand Up @@ -572,7 +573,8 @@ void assertAllSearchWorks(int count) throws IOException {

Request explainRequest = new Request("GET", "/" + index + "/" + type + "/" + id + "/_explain");
explainRequest.setJsonEntity("{ \"query\": { \"match_all\" : {} }}");
explainRequest.setOptions(expectWarnings(RestExplainAction.TYPES_DEPRECATION_MESSAGE));
explainRequest.setOptions(
expectWarnings(RestExplainAction.TYPES_DEPRECATION_MESSAGE, TypeFieldMapper.TypeFieldType.TYPES_DEPRECATION_MESSAGE));
String explanation = toStr(client().performRequest(explainRequest));
assertFalse("Could not find payload boost in explanation\n" + explanation, explanation.contains("payloadBoost"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,11 @@ public MetadataFieldMapper getDefault(MappedFieldType fieldType, ParserContext c
}
}

static final class TypeFieldType extends StringFieldType {
public static final class TypeFieldType extends StringFieldType {

private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(TypeFieldType.class));
public static final String TYPES_DEPRECATION_MESSAGE =
"[types removal] Referring to types within search queries is deprecated, filter on a field instead.";

TypeFieldType() {
}
Expand Down Expand Up @@ -124,6 +126,7 @@ public boolean isSearchable() {

@Override
public Query existsQuery(QueryShardContext context) {
Copy link
Contributor

Choose a reason for hiding this comment

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

To be really comprehensive, we could also override the other query types (fuzzy, prefix, wildcard and regex) and issue deprecation warnings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Julie. Turns out that fuzzy, prefix and regex query all fail on the _type field, as the field is not indexed. So these kind of requests are not allowed. wildcard query is calling termQuery underneath, so we should get an expected deprecation message.

deprecationLogger.deprecatedAndMaybeLog("exists_query_with_type_field", TYPES_DEPRECATION_MESSAGE);
return new MatchAllDocsQuery();
}

Expand All @@ -134,6 +137,7 @@ public Query termQuery(Object value, QueryShardContext context) {

@Override
public Query termsQuery(List<?> values, QueryShardContext context) {
deprecationLogger.deprecatedAndMaybeLog("term_query_with_type_field", TYPES_DEPRECATION_MESSAGE);
DocumentMapper mapper = context.getMapperService().documentMapper();
if (mapper == null) {
return new MatchNoDocsQuery("No types");
Expand All @@ -155,9 +159,7 @@ public Query termsQuery(List<?> values, QueryShardContext context) {

@Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, QueryShardContext context) {
deprecationLogger.deprecatedAndMaybeLog("range_single_type",
"Running [range] query on [_type] field for an index with a single type."
+ " As types are deprecated, this functionality will be removed in future releases.");
deprecationLogger.deprecatedAndMaybeLog("range_query_with_type_field", TYPES_DEPRECATION_MESSAGE);
Query result = new MatchAllDocsQuery();
String type = context.getMapperService().documentMapper().type();
if (type != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.TypeFieldMapper;

import java.io.IOException;
import java.util.Objects;
Expand Down Expand Up @@ -127,7 +128,7 @@ public String getWriteableName() {

@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
deprecationLogger.deprecated("The [type] query is deprecated, filter on a field instead.");
deprecationLogger.deprecatedAndMaybeLog("type_query", TypeFieldMapper.TypeFieldType.TYPES_DEPRECATION_MESSAGE);
//LUCENE 4 UPGRADE document mapper should use bytesref as well?
DocumentMapper documentMapper = context.getMapperService().documentMapper(type);
if (documentMapper == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,29 @@ public void testTermsQuery() throws Exception {
Mockito.when(mapperService.documentMapper()).thenReturn(mapper);
query = ft.termQuery("my_type", context);
assertEquals(new MatchNoDocsQuery(), query);
assertWarnings(TypeFieldMapper.TypeFieldType.TYPES_DEPRECATION_MESSAGE);
}

public void testExistsQuery() {
QueryShardContext context = Mockito.mock(QueryShardContext.class);
TypeFieldMapper.TypeFieldType ft = new TypeFieldMapper.TypeFieldType();
ft.setName(TypeFieldMapper.NAME);
ft.existsQuery(context);
assertWarnings(TypeFieldMapper.TypeFieldType.TYPES_DEPRECATION_MESSAGE);
}

public void testRangeQuery() {
QueryShardContext context = Mockito.mock(QueryShardContext.class);
MapperService mapperService = Mockito.mock(MapperService.class);
DocumentMapper mapper = Mockito.mock(DocumentMapper.class);
Mockito.when(context.getMapperService()).thenReturn(mapperService);
Mockito.when(mapperService.documentMapper()).thenReturn(mapper);
Mockito.when(mapper.type()).thenReturn("my_type");

TypeFieldMapper.TypeFieldType ft = new TypeFieldMapper.TypeFieldType();
ft.setName(TypeFieldMapper.NAME);
ft.rangeQuery("type1", "type2", true, true, context);
assertWarnings(TypeFieldMapper.TypeFieldType.TYPES_DEPRECATION_MESSAGE);
}

static DirectoryReader openReaderWithNewType(String type, IndexWriter writer) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ public void testFromJson() throws IOException {
@Override
public void testToQuery() throws IOException {
super.testToQuery();
assertWarnings("The [type] query is deprecated, filter on a field instead.");
assertWarnings(TypeFieldMapper.TypeFieldType.TYPES_DEPRECATION_MESSAGE);
}

@Override
public void testMustRewrite() throws IOException {
super.testMustRewrite();
assertWarnings("The [type] query is deprecated, filter on a field instead.");
assertWarnings(TypeFieldMapper.TypeFieldType.TYPES_DEPRECATION_MESSAGE);
}
}