From 13738922b0d2db2792f38731d285cfbb607078d6 Mon Sep 17 00:00:00 2001 From: javanna Date: Tue, 27 Oct 2015 11:52:32 +0100 Subject: [PATCH] Delete by query to not wrap the inner query into an additional query element The delete by query plugin used to set the provided body as the query of the SearchSourceBuilder, which means it was going to be wrapped into an additional query element. This ended up working properly only because we have a registered "query" query that makes all the parsing work anyway. That said, this has a side effect: we ended up supporting a query that is not wrapped into a query element on the REST layer, something that should not be supported. Also, we want to remove the deprecated "query" query from master as it is deprecated in 2.x, but it is not possible as long as we need it to properly parse the delete_by_query body. This is what caused #13326 in the first place, but master has changed in the meantime and will need different changes. Relates to #13326 --- .../test/delete_by_query/10_basic.yaml | 16 ++++++++++++++-- .../TransportDeleteByQueryAction.java | 10 +++------- .../TransportDeleteByQueryActionTests.java | 3 ++- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/plugins/delete-by-query/rest-api-spec/test/delete_by_query/10_basic.yaml b/plugins/delete-by-query/rest-api-spec/test/delete_by_query/10_basic.yaml index c253ad8d27659..7bbc6fbdcb092 100644 --- a/plugins/delete-by-query/rest-api-spec/test/delete_by_query/10_basic.yaml +++ b/plugins/delete-by-query/rest-api-spec/test/delete_by_query/10_basic.yaml @@ -1,5 +1,4 @@ ---- -"Basic delete_by_query": +setup: - do: index: index: test_1 @@ -24,6 +23,8 @@ - do: indices.refresh: {} +--- +"Basic delete_by_query": - do: delete_by_query: index: test_1 @@ -40,3 +41,14 @@ index: test_1 - match: { count: 2 } + +--- +"No query element delete_by_query": + - do: + catch: request + delete_by_query: + index: test_1 + body: + match: + foo: bar + diff --git a/plugins/delete-by-query/src/main/java/org/elasticsearch/action/deletebyquery/TransportDeleteByQueryAction.java b/plugins/delete-by-query/src/main/java/org/elasticsearch/action/deletebyquery/TransportDeleteByQueryAction.java index 602b0a4cabe71..fcb8707c30c1e 100644 --- a/plugins/delete-by-query/src/main/java/org/elasticsearch/action/deletebyquery/TransportDeleteByQueryAction.java +++ b/plugins/delete-by-query/src/main/java/org/elasticsearch/action/deletebyquery/TransportDeleteByQueryAction.java @@ -106,15 +106,15 @@ void executeScan() { if (request.routing() != null) { scanRequest.routing(request.routing()); } - - SearchSourceBuilder source = new SearchSourceBuilder().query(request.source()).fields("_routing", "_parent").fetchSource(false).version(true); + scanRequest.source(request.source()); + SearchSourceBuilder source = new SearchSourceBuilder().fields("_routing", "_parent").fetchSource(false).version(true); if (request.size() > 0) { source.size(request.size()); } if (request.timeout() != null) { source.timeout(request.timeout()); } - scanRequest.source(source); + scanRequest.extraSource(source); logger.trace("executing scan request"); searchAction.execute(scanRequest, new ActionListener() { @@ -299,10 +299,6 @@ boolean hasTimedOut() { return request.timeout() != null && (threadPool.estimatedTimeInMillis() >= (startTime + request.timeout().millis())); } - void addShardFailure(ShardOperationFailedException failure) { - addShardFailures(new ShardOperationFailedException[]{failure}); - } - void addShardFailures(ShardOperationFailedException[] failures) { if (!CollectionUtils.isEmpty(failures)) { ShardOperationFailedException[] duplicates = new ShardOperationFailedException[shardFailures.length + failures.length]; diff --git a/plugins/delete-by-query/src/test/java/org/elasticsearch/action/deletebyquery/TransportDeleteByQueryActionTests.java b/plugins/delete-by-query/src/test/java/org/elasticsearch/action/deletebyquery/TransportDeleteByQueryActionTests.java index f1edc8d8b8ad0..eebf73c0dd12e 100644 --- a/plugins/delete-by-query/src/test/java/org/elasticsearch/action/deletebyquery/TransportDeleteByQueryActionTests.java +++ b/plugins/delete-by-query/src/test/java/org/elasticsearch/action/deletebyquery/TransportDeleteByQueryActionTests.java @@ -30,6 +30,7 @@ import org.elasticsearch.action.search.ClearScrollResponse; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.action.support.QuerySourceBuilder; import org.elasticsearch.common.Strings; import org.elasticsearch.common.text.StringText; import org.elasticsearch.common.unit.TimeValue; @@ -85,7 +86,7 @@ public void testExecuteScan() { assertHitCount(client().prepareCount("test").get(), numDocs); final long limit = randomIntBetween(0, numDocs); - DeleteByQueryRequest delete = new DeleteByQueryRequest().indices("test").source(boolQuery().must(rangeQuery("num").lte(limit)).buildAsBytes()); + DeleteByQueryRequest delete = new DeleteByQueryRequest().indices("test").source(new QuerySourceBuilder().setQuery(boolQuery().must(rangeQuery("num").lte(limit)))); TestActionListener listener = new TestActionListener(); newAsyncAction(delete, listener).executeScan();