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 elastic#13326 in the first place, but master has changed in the meantime and will need different changes.

Relates to elastic#13326
  • Loading branch information
javanna committed Oct 27, 2015
1 parent ab99b60 commit 1373892
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 10 deletions.
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

Original file line number Diff line number Diff line change
Expand Up @@ -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<SearchResponse>() {
Expand Down Expand Up @@ -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];
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

0 comments on commit 1373892

Please sign in to comment.