Make the `type` parameter optional when percolating existing documents. (#39987) (#39989)

`document_type` is the type to use for parsing the document to percolate, which
is already optional and deprecated. However `percotale` queries also have the
ability to percolate existing documents, identified by an index, an id and a
type. This change makes the latter optional and deprecated.

Closes #39963
This commit is contained in:
Adrien Grand 2019-03-13 15:04:41 +01:00 committed by GitHub
parent 16e4499b97
commit 9731ba4338
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 202 additions and 27 deletions

View File

@ -134,7 +134,7 @@ The following parameters are required when percolating a document:
This is an optional parameter. This is an optional parameter.
`document`:: The source of the document being percolated. `document`:: The source of the document being percolated.
`documents`:: Like the `document` parameter, but accepts multiple documents via a json array. `documents`:: Like the `document` parameter, but accepts multiple documents via a json array.
`document_type`:: The type / mapping of the document being percolated. This setting is deprecated and only required for indices created before 6.0 `document_type`:: The type / mapping of the document being percolated. This parameter is deprecated and will be removed in Elasticsearch 8.0.
Instead of specifying the source of the document being percolated, the source can also be retrieved from an already Instead of specifying the source of the document being percolated, the source can also be retrieved from an already
stored document. The `percolate` query will then internally execute a get request to fetch that document. stored document. The `percolate` query will then internally execute a get request to fetch that document.
@ -143,7 +143,7 @@ In that case the `document` parameter can be substituted with the following para
[horizontal] [horizontal]
`index`:: The index the document resides in. This is a required parameter. `index`:: The index the document resides in. This is a required parameter.
`type`:: The type of the document to fetch. This is a required parameter. `type`:: The type of the document to fetch. This parameter is deprecated and will be removed in Elasticsearch 8.0.
`id`:: The id of the document to fetch. This is a required parameter. `id`:: The id of the document to fetch. This is a required parameter.
`routing`:: Optionally, routing to be used to fetch document to percolate. `routing`:: Optionally, routing to be used to fetch document to percolate.
`preference`:: Optionally, preference to be used to fetch document to percolate. `preference`:: Optionally, preference to be used to fetch document to percolate.
@ -323,7 +323,6 @@ GET /my-index/_search
"percolate" : { "percolate" : {
"field": "query", "field": "query",
"index" : "my-index", "index" : "my-index",
"type" : "_doc",
"id" : "2", "id" : "2",
"version" : 1 <1> "version" : 1 <1>
} }

View File

@ -96,6 +96,10 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
public static final String NAME = "percolate"; public static final String NAME = "percolate";
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(ParseField.class)); private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(ParseField.class));
static final String DOCUMENT_TYPE_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [percolate] queries. " +
"The [document_type] should no longer be specified.";
static final String TYPE_DEPRECATION_MESSAGE = "[types removal] Types are deprecated in [percolate] queries. " +
"The [type] of the indexed document should no longer be specified.";
static final ParseField DOCUMENT_FIELD = new ParseField("document"); static final ParseField DOCUMENT_FIELD = new ParseField("document");
static final ParseField DOCUMENTS_FIELD = new ParseField("documents"); static final ParseField DOCUMENTS_FIELD = new ParseField("documents");
@ -117,6 +121,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
private final XContentType documentXContentType; private final XContentType documentXContentType;
private final String indexedDocumentIndex; private final String indexedDocumentIndex;
@Deprecated
private final String indexedDocumentType; private final String indexedDocumentType;
private final String indexedDocumentId; private final String indexedDocumentId;
private final String indexedDocumentRouting; private final String indexedDocumentRouting;
@ -220,9 +225,6 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
if (indexedDocumentIndex == null) { if (indexedDocumentIndex == null) {
throw new IllegalArgumentException("[index] is a required argument"); throw new IllegalArgumentException("[index] is a required argument");
} }
if (indexedDocumentType == null) {
throw new IllegalArgumentException("[type] is a required argument");
}
if (indexedDocumentId == null) { if (indexedDocumentId == null) {
throw new IllegalArgumentException("[id] is a required argument"); throw new IllegalArgumentException("[id] is a required argument");
} }
@ -521,7 +523,13 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
XContentHelper.xContentType(source)); XContentHelper.xContentType(source));
} }
} }
GetRequest getRequest = new GetRequest(indexedDocumentIndex, indexedDocumentType, indexedDocumentId); GetRequest getRequest;
if (indexedDocumentType != null) {
deprecationLogger.deprecatedAndMaybeLog("percolate_with_type", TYPE_DEPRECATION_MESSAGE);
getRequest = new GetRequest(indexedDocumentIndex, indexedDocumentType, indexedDocumentId);
} else {
getRequest = new GetRequest(indexedDocumentIndex, indexedDocumentId);
}
getRequest.preference("_local"); getRequest.preference("_local");
getRequest.routing(indexedDocumentRouting); getRequest.routing(indexedDocumentRouting);
getRequest.preference(indexedDocumentPreference); getRequest.preference(indexedDocumentPreference);
@ -533,13 +541,14 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
client.get(getRequest, ActionListener.wrap(getResponse -> { client.get(getRequest, ActionListener.wrap(getResponse -> {
if (getResponse.isExists() == false) { if (getResponse.isExists() == false) {
throw new ResourceNotFoundException( throw new ResourceNotFoundException(
"indexed document [{}/{}/{}] couldn't be found", indexedDocumentIndex, indexedDocumentType, indexedDocumentId "indexed document [{}{}/{}] couldn't be found", indexedDocumentIndex,
indexedDocumentType == null ? "" : "/" + indexedDocumentType, indexedDocumentId
); );
} }
if(getResponse.isSourceEmpty()) { if(getResponse.isSourceEmpty()) {
throw new IllegalArgumentException( throw new IllegalArgumentException(
"indexed document [" + indexedDocumentIndex + "/" + indexedDocumentType + "/" + indexedDocumentId "indexed document [" + indexedDocumentIndex + (indexedDocumentType == null ? "" : "/" + indexedDocumentType) +
+ "] source disabled" "/" + indexedDocumentId + "] source disabled"
); );
} }
documentSupplier.set(getResponse.getSourceAsBytesRef()); documentSupplier.set(getResponse.getSourceAsBytesRef());
@ -554,7 +563,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
// Call nowInMillis() so that this query becomes un-cacheable since we // Call nowInMillis() so that this query becomes un-cacheable since we
// can't be sure that it doesn't use now or scripts // can't be sure that it doesn't use now or scripts
context.nowInMillis(); context.nowInMillis();
if (indexedDocumentIndex != null || indexedDocumentType != null || indexedDocumentId != null || documentSupplier != null) { if (indexedDocumentIndex != null || indexedDocumentId != null || documentSupplier != null) {
throw new IllegalStateException("query builder must be rewritten first"); throw new IllegalStateException("query builder must be rewritten first");
} }
@ -577,7 +586,7 @@ public class PercolateQueryBuilder extends AbstractQueryBuilder<PercolateQueryBu
final MapperService mapperService = context.getMapperService(); final MapperService mapperService = context.getMapperService();
String type = mapperService.documentMapper().type(); String type = mapperService.documentMapper().type();
if (documentType != null) { if (documentType != null) {
deprecationLogger.deprecated("[document_type] parameter has been deprecated because types have been deprecated"); deprecationLogger.deprecatedAndMaybeLog("percolate_with_document_type", DOCUMENT_TYPE_DEPRECATION_MESSAGE);
if (documentType.equals(type) == false) { if (documentType.equals(type) == false) {
throw new IllegalArgumentException("specified document_type [" + documentType + throw new IllegalArgumentException("specified document_type [" + documentType +
"] is not equal to the actual type [" + type + "]"); "] is not equal to the actual type [" + type + "]");

View File

@ -29,6 +29,7 @@ import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.lucene.uid.Versions;
import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.XContentType;
@ -69,7 +70,6 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase<PercolateQ
private static String docType; private static String docType;
private String indexedDocumentIndex; private String indexedDocumentIndex;
private String indexedDocumentType;
private String indexedDocumentId; private String indexedDocumentId;
private String indexedDocumentRouting; private String indexedDocumentRouting;
private String indexedDocumentPreference; private String indexedDocumentPreference;
@ -88,7 +88,7 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase<PercolateQ
queryField = randomAlphaOfLength(4); queryField = randomAlphaOfLength(4);
aliasField = randomAlphaOfLength(4); aliasField = randomAlphaOfLength(4);
String docType = "_doc"; docType = "_doc";
mapperService.merge(docType, new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef(docType, mapperService.merge(docType, new CompressedXContent(Strings.toString(PutMappingRequest.buildFromSimplifiedDef(docType,
queryField, "type=percolator", aliasField, "type=alias,path=" + queryField queryField, "type=percolator", aliasField, "type=alias,path=" + queryField
))), MapperService.MergeReason.MAPPING_UPDATE); ))), MapperService.MergeReason.MAPPING_UPDATE);
@ -117,15 +117,14 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase<PercolateQ
PercolateQueryBuilder queryBuilder; PercolateQueryBuilder queryBuilder;
if (indexedDocument) { if (indexedDocument) {
indexedDocumentIndex = randomAlphaOfLength(4); indexedDocumentIndex = randomAlphaOfLength(4);
indexedDocumentType = "doc";
indexedDocumentId = randomAlphaOfLength(4); indexedDocumentId = randomAlphaOfLength(4);
indexedDocumentRouting = randomAlphaOfLength(4); indexedDocumentRouting = randomAlphaOfLength(4);
indexedDocumentPreference = randomAlphaOfLength(4); indexedDocumentPreference = randomAlphaOfLength(4);
indexedDocumentVersion = (long) randomIntBetween(0, Integer.MAX_VALUE); indexedDocumentVersion = (long) randomIntBetween(0, Integer.MAX_VALUE);
queryBuilder = new PercolateQueryBuilder(queryField, docType, indexedDocumentIndex, indexedDocumentType, indexedDocumentId, queryBuilder = new PercolateQueryBuilder(queryField, null, indexedDocumentIndex, null, indexedDocumentId,
indexedDocumentRouting, indexedDocumentPreference, indexedDocumentVersion); indexedDocumentRouting, indexedDocumentPreference, indexedDocumentVersion);
} else { } else {
queryBuilder = new PercolateQueryBuilder(queryField, docType, documentSource, XContentType.JSON); queryBuilder = new PercolateQueryBuilder(queryField, null, documentSource, XContentType.JSON);
} }
if (randomBoolean()) { if (randomBoolean()) {
queryBuilder.setName(randomAlphaOfLength(4)); queryBuilder.setName(randomAlphaOfLength(4));
@ -146,19 +145,19 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase<PercolateQ
@Override @Override
protected GetResponse executeGet(GetRequest getRequest) { protected GetResponse executeGet(GetRequest getRequest) {
assertThat(getRequest.index(), Matchers.equalTo(indexedDocumentIndex)); assertThat(getRequest.index(), Matchers.equalTo(indexedDocumentIndex));
assertThat(getRequest.type(), Matchers.equalTo(indexedDocumentType)); assertThat(getRequest.type(), Matchers.equalTo(MapperService.SINGLE_MAPPING_NAME));
assertThat(getRequest.id(), Matchers.equalTo(indexedDocumentId)); assertThat(getRequest.id(), Matchers.equalTo(indexedDocumentId));
assertThat(getRequest.routing(), Matchers.equalTo(indexedDocumentRouting)); assertThat(getRequest.routing(), Matchers.equalTo(indexedDocumentRouting));
assertThat(getRequest.preference(), Matchers.equalTo(indexedDocumentPreference)); assertThat(getRequest.preference(), Matchers.equalTo(indexedDocumentPreference));
assertThat(getRequest.version(), Matchers.equalTo(indexedDocumentVersion)); assertThat(getRequest.version(), Matchers.equalTo(indexedDocumentVersion));
if (indexedDocumentExists) { if (indexedDocumentExists) {
return new GetResponse( return new GetResponse(
new GetResult(indexedDocumentIndex, indexedDocumentType, indexedDocumentId, 0, 1, 0L, true, new GetResult(indexedDocumentIndex, MapperService.SINGLE_MAPPING_NAME, indexedDocumentId, 0, 1, 0L, true,
documentSource.iterator().next(), Collections.emptyMap()) documentSource.iterator().next(), Collections.emptyMap())
); );
} else { } else {
return new GetResponse( return new GetResponse(
new GetResult(indexedDocumentIndex, indexedDocumentType, indexedDocumentId, UNASSIGNED_SEQ_NO, 0, -1, new GetResult(indexedDocumentIndex, MapperService.SINGLE_MAPPING_NAME, indexedDocumentId, UNASSIGNED_SEQ_NO, 0, -1,
false, null, Collections.emptyMap()) false, null, Collections.emptyMap())
); );
} }
@ -168,7 +167,7 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase<PercolateQ
protected void doAssertLuceneQuery(PercolateQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { protected void doAssertLuceneQuery(PercolateQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException {
assertThat(query, Matchers.instanceOf(PercolateQuery.class)); assertThat(query, Matchers.instanceOf(PercolateQuery.class));
PercolateQuery percolateQuery = (PercolateQuery) query; PercolateQuery percolateQuery = (PercolateQuery) query;
assertThat(docType, Matchers.equalTo(queryBuilder.getDocumentType())); assertNull(queryBuilder.getDocumentType());
assertThat(percolateQuery.getDocuments(), Matchers.equalTo(documentSource)); assertThat(percolateQuery.getDocuments(), Matchers.equalTo(documentSource));
} }
@ -188,7 +187,7 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase<PercolateQ
PercolateQueryBuilder pqb = doCreateTestQueryBuilder(true); PercolateQueryBuilder pqb = doCreateTestQueryBuilder(true);
ResourceNotFoundException e = expectThrows(ResourceNotFoundException.class, () -> rewriteAndFetch(pqb, ResourceNotFoundException e = expectThrows(ResourceNotFoundException.class, () -> rewriteAndFetch(pqb,
createShardContext())); createShardContext()));
String expectedString = "indexed document [" + indexedDocumentIndex + "/" + indexedDocumentType + "/" + String expectedString = "indexed document [" + indexedDocumentIndex + "/" +
indexedDocumentId + "] couldn't be found"; indexedDocumentId + "] couldn't be found";
assertThat(e.getMessage() , equalTo(expectedString)); assertThat(e.getMessage() , equalTo(expectedString));
} }
@ -220,11 +219,6 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase<PercolateQ
}); });
assertThat(e.getMessage(), equalTo("[index] is a required argument")); assertThat(e.getMessage(), equalTo("[index] is a required argument"));
e = expectThrows(IllegalArgumentException.class, () -> {
new PercolateQueryBuilder("_field", "_document_type", "_index", null, "_id", null, null, null);
});
assertThat(e.getMessage(), equalTo("[type] is a required argument"));
e = expectThrows(IllegalArgumentException.class, () -> { e = expectThrows(IllegalArgumentException.class, () -> {
new PercolateQueryBuilder("_field", "_document_type", "_index", "_type", null, null, null, null); new PercolateQueryBuilder("_field", "_document_type", "_index", "_type", null, null, null, null);
}); });
@ -237,6 +231,39 @@ public class PercolateQueryBuilderTests extends AbstractQueryTestCase<PercolateQ
queryBuilder.toQuery(queryShardContext); queryBuilder.toQuery(queryShardContext);
} }
public void testFromJsonWithDocumentType() throws IOException {
QueryShardContext queryShardContext = createShardContext();
QueryBuilder queryBuilder = parseQuery("{\"percolate\" : { \"document\": {}, \"document_type\":\"" + docType + "\", \"field\":\"" +
queryField + "\"}}");
queryBuilder.toQuery(queryShardContext);
assertWarnings(PercolateQueryBuilder.DOCUMENT_TYPE_DEPRECATION_MESSAGE);
}
public void testFromJsonNoType() throws IOException {
indexedDocumentIndex = randomAlphaOfLength(4);
indexedDocumentId = randomAlphaOfLength(4);
indexedDocumentVersion = Versions.MATCH_ANY;
documentSource = Collections.singletonList(randomSource(new HashSet<>()));
QueryShardContext queryShardContext = createShardContext();
QueryBuilder queryBuilder = parseQuery("{\"percolate\" : { \"index\": \"" + indexedDocumentIndex + "\", \"id\": \"" +
indexedDocumentId + "\", \"field\":\"" + queryField + "\"}}");
rewriteAndFetch(queryBuilder, queryShardContext).toQuery(queryShardContext);
}
public void testFromJsonWithType() throws IOException {
indexedDocumentIndex = randomAlphaOfLength(4);
indexedDocumentId = randomAlphaOfLength(4);
indexedDocumentVersion = Versions.MATCH_ANY;
documentSource = Collections.singletonList(randomSource(new HashSet<>()));
QueryShardContext queryShardContext = createShardContext();
QueryBuilder queryBuilder = parseQuery("{\"percolate\" : { \"index\": \"" + indexedDocumentIndex +
"\", \"type\": \"_doc\", \"id\": \"" + indexedDocumentId + "\", \"field\":\"" + queryField + "\"}}");
rewriteAndFetch(queryBuilder, queryShardContext).toQuery(queryShardContext);
assertWarnings(PercolateQueryBuilder.TYPE_DEPRECATION_MESSAGE);
}
public void testBothDocumentAndDocumentsSpecified() throws IOException { public void testBothDocumentAndDocumentsSpecified() throws IOException {
expectThrows(IllegalArgumentException.class, expectThrows(IllegalArgumentException.class,
() -> parseQuery("{\"percolate\" : { \"document\": {}, \"documents\": [{}, {}], \"field\":\"" + queryField + "\"}}")); () -> parseQuery("{\"percolate\" : { \"document\": {}, \"documents\": [{}, {}], \"field\":\"" + queryField + "\"}}"));

View File

@ -1,5 +1,10 @@
--- ---
"Test percolator basics via rest": "Test percolator basics via rest":
- skip:
version: " - 6.99.99"
reason: types are required in requests before 7.0.0
- do: - do:
indices.create: indices.create:
index: queries_index index: queries_index
@ -11,6 +16,15 @@
foo: foo:
type: keyword type: keyword
- do:
indices.create:
index: documents_index
body:
mappings:
properties:
foo:
type: keyword
- do: - do:
index: index:
index: queries_index index: queries_index
@ -19,6 +33,13 @@
query: query:
match_all: {} match_all: {}
- do:
index:
index: documents_index
id: some_id
body:
foo: bar
- do: - do:
indices.refresh: {} indices.refresh: {}
@ -44,3 +65,26 @@
document: document:
foo: bar foo: bar
- match: { responses.0.hits.total: 1 } - match: { responses.0.hits.total: 1 }
- do:
search:
rest_total_hits_as_int: true
body:
- query:
percolate:
field: query
index: documents_index
id: some_id
- match: { hits.total: 1 }
- do:
msearch:
rest_total_hits_as_int: true
body:
- index: queries_index
- query:
percolate:
field: query
index: documents_index
id: some_id
- match: { responses.0.hits.total: 1 }

View File

@ -0,0 +1,96 @@
---
"Test percolator basics via rest":
- do:
indices.create:
include_type_name: true
index: queries_index
body:
mappings:
queries_type:
properties:
query:
type: percolator
foo:
type: keyword
- do:
indices.create:
include_type_name: true
index: documents_index
body:
mappings:
documents_type:
properties:
foo:
type: keyword
- do:
index:
index: queries_index
type: queries_type
id: test_percolator
body:
query:
match_all: {}
- do:
index:
index: documents_index
type: documents_type
id: some_id
body:
foo: bar
- do:
indices.refresh: {}
- do:
search:
rest_total_hits_as_int: true
body:
- query:
percolate:
field: query
document:
document_type: queries_type
foo: bar
- match: { hits.total: 1 }
- do:
msearch:
rest_total_hits_as_int: true
body:
- index: queries_index
- query:
percolate:
field: query
document_type: queries_type
document:
foo: bar
- match: { responses.0.hits.total: 1 }
- do:
search:
rest_total_hits_as_int: true
body:
- query:
percolate:
field: query
index: documents_index
type: documents_type
id: some_id
- match: { hits.total: 1 }
- do:
msearch:
rest_total_hits_as_int: true
body:
- index: queries_index
- query:
percolate:
field: query
index: documents_index
type: documents_type
id: some_id
- match: { responses.0.hits.total: 1 }