From 4c47daa8d608b9f61fde57fcf452fe8bee9cdd7a Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Wed, 20 Apr 2022 08:58:19 +0200 Subject: [PATCH] Rewrite match_all inside must_not (#85999) A must_not with a match_all clause inside a bool query is currently not rewritten to a match_none query. This means that running a boolean query with "must_not":[{"terms":{"_tier":["data_frozen","data_cold"]}] is currently not rewritten as match_none on a cold/frozen tier node. --- docs/changelog/85999.yaml | 5 +++++ .../org/elasticsearch/upgrades/QueryBuilderBWCIT.java | 5 +++-- .../elasticsearch/index/query/BoolQueryBuilder.java | 11 ++++------- .../index/query/BoolQueryBuilderTests.java | 6 ++++++ 4 files changed, 18 insertions(+), 9 deletions(-) create mode 100644 docs/changelog/85999.yaml diff --git a/docs/changelog/85999.yaml b/docs/changelog/85999.yaml new file mode 100644 index 000000000000..1fa1f2fd034b --- /dev/null +++ b/docs/changelog/85999.yaml @@ -0,0 +1,5 @@ +pr: 85999 +summary: Rewrite `match_all` inside `must_not` +area: Search +type: bug +issues: [] diff --git a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java index ff27ecfc3851..129030f73113 100644 --- a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java +++ b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/QueryBuilderBWCIT.java @@ -21,6 +21,7 @@ import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.ConstantScoreQueryBuilder; import org.elasticsearch.index.query.DisMaxQueryBuilder; import org.elasticsearch.index.query.MatchAllQueryBuilder; +import org.elasticsearch.index.query.MatchNoneQueryBuilder; import org.elasticsearch.index.query.MatchPhraseQueryBuilder; import org.elasticsearch.index.query.MatchQueryBuilder; import org.elasticsearch.index.query.Operator; @@ -84,10 +85,10 @@ public class QueryBuilderBWCIT extends AbstractFullClusterRestartTestCase { """, new RangeQueryBuilder("long_field").from(1).to(9)); addCandidate( """ - "bool": { "must_not": [{"match_all": {}}], "must": [{"match_all": {}}], "filter": [{"match_all": {}}], \ + "bool": { "must_not": [{"match_none": {}}], "must": [{"match_all": {}}], "filter": [{"match_all": {}}], \ "should": [{"match_all": {}}]} """, - new BoolQueryBuilder().mustNot(new MatchAllQueryBuilder()) + new BoolQueryBuilder().mustNot(new MatchNoneQueryBuilder()) .must(new MatchAllQueryBuilder()) .filter(new MatchAllQueryBuilder()) .should(new MatchAllQueryBuilder()) diff --git a/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java index cb338fecc7f3..cf5a7fe58f4f 100644 --- a/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -29,9 +29,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.function.Consumer; -import java.util.stream.Stream; import static org.elasticsearch.common.lucene.search.Queries.fixNegativeQueryIfNeeded; import static org.elasticsearch.search.SearchModule.INDICES_MAX_NESTED_DEPTH_SETTING; @@ -385,11 +383,10 @@ public class BoolQueryBuilder extends AbstractQueryBuilder { } // lets do some early termination and prevent any kind of rewriting if we have a mandatory query that is a MatchNoneQueryBuilder - Optional any = Stream.concat(newBuilder.mustClauses.stream(), newBuilder.filterClauses.stream()) - .filter(b -> b instanceof MatchNoneQueryBuilder) - .findAny(); - if (any.isPresent()) { - return any.get(); + if (newBuilder.mustClauses.stream().anyMatch(b -> b instanceof MatchNoneQueryBuilder) + || newBuilder.filterClauses.stream().anyMatch(b -> b instanceof MatchNoneQueryBuilder) + || newBuilder.mustNotClauses.stream().anyMatch(b -> b instanceof MatchAllQueryBuilder)) { + return new MatchNoneQueryBuilder(); } if (changed) { diff --git a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index eaf2090cad7a..bd3e5f53b623 100644 --- a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -425,6 +425,12 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase