Skip to content

Commit

Permalink
Delete by query to not wrap the inner query into an additional query …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
javanna committed Oct 27, 2015
1 parent b460535 commit da6ec73
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ void executeScan() {
scanRequest.routing(request.routing());
}

scanRequest.source(request.source());
SearchSourceBuilder source = new SearchSourceBuilder()
.query(request.source())
.fields("_routing", "_parent")
.sort("_doc") // important for performance
.fetchSource(false)
Expand All @@ -119,7 +119,7 @@ void executeScan() {
if (request.timeout() != null) {
source.timeout(request.timeout());
}
scanRequest.source(source);
scanRequest.extraSource(source);

logger.trace("executing scan request");
searchAction.execute(scanRequest, new ActionListener<SearchResponse>() {
Expand Down Expand Up @@ -302,10 +302,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];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
---
"Basic delete_by_query":
setup:
- do:
index:
index: test_1
Expand All @@ -24,6 +23,8 @@
- do:
indices.refresh: {}

---
"Basic delete_by_query":
- do:
delete_by_query:
index: test_1
Expand All @@ -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

0 comments on commit da6ec73

Please sign in to comment.