diff --git a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/70_skip_shards.yml b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/70_skip_shards.yml index e50b32c5e4b3c..7a00bc6990e99 100644 --- a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/70_skip_shards.yml +++ b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/70_skip_shards.yml @@ -53,3 +53,166 @@ - match: { _shards.failed: 0 } - match: { hits.total: 1 } +--- +"Test that queries on _index field that don't match alias are skipped": + + - do: + indices.create: + index: skip_shards_local_index + body: + settings: + index: + number_of_shards: 2 + number_of_replicas: 0 + mappings: + properties: + created_at: + type: date + format: "yyyy-MM-dd" + + - do: + bulk: + refresh: true + body: + - '{"index": {"_index": "skip_shards_local_index"}}' + - '{"f1": "local_cluster", "sort_field": 0, "created_at" : "2017-01-01"}' + - '{"index": {"_index": "skip_shards_local_index"}}' + - '{"f1": "local_cluster", "sort_field": 1, "created_at" : "2017-01-02"}' + - do: + indices.put_alias: + index: skip_shards_local_index + name: test_skip_alias + + # check that we match the alias with term query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "term" : { "_index" : "test_skip_alias" } } } + + - match: { hits.total.value: 2 } + - match: { hits.hits.0._index: "skip_shards_local_index"} + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 0} + - match: { _shards.failed: 0 } + + # check that we match the alias with terms query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "terms" : { "_index" : ["test_skip_alias", "does_not_match"] } } } + + - match: { hits.total.value: 2 } + - match: { hits.hits.0._index: "skip_shards_local_index"} + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 0} + - match: { _shards.failed: 0 } + + # check that we match the alias with prefix query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "prefix" : { "_index" : "test_skip_ali" } } } + + - match: { hits.total.value: 2 } + - match: { hits.hits.0._index: "skip_shards_local_index"} + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 0} + - match: { _shards.failed: 0 } + + # check that we match the alias with wildcard query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "wildcard" : { "_index" : "test_skip_ali*" } } } + + - match: { hits.total.value: 2 } + - match: { hits.hits.0._index: "skip_shards_local_index"} + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 0} + - match: { _shards.failed: 0 } + + + # check that skipped when we don't match the alias with a term query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "term" : { "_index" : "does_not_match" } } } + + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + # When all shards are skipped current logic returns 1 to produce a valid search result + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } + + # check that skipped when we don't match the alias with a terms query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "terms" : { "_index" : ["does_not_match", "also_does_not_match"] } } } + + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + # When all shards are skipped current logic returns 1 to produce a valid search result + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } + + # check that skipped when we don't match the alias with a prefix query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "prefix" : { "_index" : "does_not_matc" } } } + + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + # When all shards are skipped current logic returns 1 to produce a valid search result + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } + + # check that skipped when we don't match the alias with a wildcard query + - do: + search: + track_total_hits: true + index: "skip_shards_local_index" + pre_filter_shard_size: 1 + ccs_minimize_roundtrips: false + body: { "size" : 10, "query" : { "wildcard" : { "_index" : "does_not_matc*" } } } + + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + # When all shards are skipped current logic returns 1 to produce a valid search result + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } + diff --git a/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/90_index_name_query.yml b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/90_index_name_query.yml new file mode 100644 index 0000000000000..a60a1b0d812ee --- /dev/null +++ b/qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/90_index_name_query.yml @@ -0,0 +1,102 @@ +--- +setup: + - do: + indices.create: + index: single_doc_index + body: + settings: + index: + number_of_shards: 1 + number_of_replicas: 0 +--- +teardown: + - do: + indices.delete: + index: single_doc_index + ignore_unavailable: true + +--- +"Test that queries on _index match against the correct indices.": + + - do: + bulk: + refresh: true + body: + - '{"index": {"_index": "single_doc_index"}}' + - '{"f1": "local_cluster", "sort_field": 0}' + + - do: + search: + rest_total_hits_as_int: true + index: "single_doc_index,my_remote_cluster:single_doc_index" + body: + query: + term: + "_index": "single_doc_index" + + - match: { hits.total: 1 } + - match: { hits.hits.0._index: "single_doc_index"} + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 0} + - match: { _shards.failed: 0 } + + - do: + search: + rest_total_hits_as_int: true + index: "single_doc_index,my_remote_cluster:single_doc_index" + body: + query: + term: + "_index": "my_remote_cluster:single_doc_index" + + - match: { hits.total: 1 } + - match: { hits.hits.0._index: "my_remote_cluster:single_doc_index"} + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 0} + - match: { _shards.failed: 0 } + +--- +"Test that queries on _index that don't match are skipped": + + - do: + bulk: + refresh: true + body: + - '{"index": {"_index": "single_doc_index"}}' + - '{"f1": "local_cluster", "sort_field": 0}' + + - do: + search: + ccs_minimize_roundtrips: false + track_total_hits: true + index: "single_doc_index,my_remote_cluster:single_doc_index" + pre_filter_shard_size: 1 + body: + query: + term: + "_index": "does_not_match" + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } + + - do: + search: + ccs_minimize_roundtrips: false + track_total_hits: true + index: "single_doc_index,my_remote_cluster:single_doc_index" + pre_filter_shard_size: 1 + body: + query: + term: + "_index": "my_remote_cluster:does_not_match" + + - match: { hits.total.value: 0 } + - match: { _shards.total: 2 } + - match: { _shards.successful: 2 } + - match: { _shards.skipped : 1} + - match: { _shards.failed: 0 } diff --git a/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java index eacb2be100c98..db596e2ecfc7b 100644 --- a/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java @@ -168,6 +168,19 @@ public static PrefixQueryBuilder fromXContent(XContentParser parser) throws IOEx public String getWriteableName() { return NAME; } + + @Override + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + if ("_index".equals(fieldName)) { + // Special-case optimisation for canMatch phase: + // We can skip querying this shard if the index name doesn't match the value of this query on the "_index" field. + QueryShardContext shardContext = queryRewriteContext.convertToShardContext(); + if (shardContext != null && shardContext.indexMatches(value + "*") == false) { + return new MatchNoneQueryBuilder(); + } + } + return super.doRewrite(queryRewriteContext); + } @Override protected Query doToQuery(QueryShardContext context) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java index c35aa9b03d581..262bfb2c6b5b3 100644 --- a/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/TermQueryBuilder.java @@ -129,6 +129,19 @@ public static TermQueryBuilder fromXContent(XContentParser parser) throws IOExce } return termQuery; } + + @Override + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + if ("_index".equals(fieldName)) { + // Special-case optimisation for canMatch phase: + // We can skip querying this shard if the index name doesn't match the value of this query on the "_index" field. + QueryShardContext shardContext = queryRewriteContext.convertToShardContext(); + if (shardContext != null && shardContext.indexMatches(BytesRefs.toString(value)) == false) { + return new MatchNoneQueryBuilder(); + } + } + return super.doRewrite(queryRewriteContext); + } @Override protected Query doToQuery(QueryShardContext context) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java index 4b4022bbab17a..0d5521104faee 100644 --- a/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/TermsQueryBuilder.java @@ -493,6 +493,21 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) { }); return new TermsQueryBuilder(this.fieldName, supplier::get); } + if ("_index".equals(this.fieldName) && values != null) { + // Special-case optimisation for canMatch phase: + // We can skip querying this shard if the index name doesn't match any of the search terms. + QueryShardContext shardContext = queryRewriteContext.convertToShardContext(); + if (shardContext != null) { + for (Object localValue : values) { + if (shardContext.indexMatches(BytesRefs.toString(localValue))) { + // We can match - at least one index name matches + return this; + } + } + // all index names are invalid - no possibility of a match on this shard. + return new MatchNoneQueryBuilder(); + } + } return this; } } diff --git a/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java index 0b855bd50a426..115fa8d476dfd 100644 --- a/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.lucene.BytesRefs; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -177,7 +178,20 @@ public static WildcardQueryBuilder fromXContent(XContentParser parser) throws IO .rewrite(rewrite) .boost(boost) .queryName(queryName); - } + } + + @Override + protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException { + if ("_index".equals(fieldName)) { + // Special-case optimisation for canMatch phase: + // We can skip querying this shard if the index name doesn't match the value of this query on the "_index" field. + QueryShardContext shardContext = queryRewriteContext.convertToShardContext(); + if (shardContext != null && shardContext.indexMatches(BytesRefs.toString(value)) == false) { + return new MatchNoneQueryBuilder(); + } + } + return super.doRewrite(queryRewriteContext); + } @Override protected Query doToQuery(QueryShardContext context) throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java index c78c83c154ff2..dc80d23e8339e 100644 --- a/server/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/PrefixQueryBuilderTests.java @@ -143,4 +143,19 @@ public void testParseFailsWithMultipleFields() throws IOException { e = expectThrows(ParsingException.class, () -> parseQuery(shortJson)); assertEquals("[prefix] query doesn't support multiple fields, found [user1] and [user2]", e.getMessage()); } + + public void testRewriteIndexQueryToMatchNone() throws Exception { + PrefixQueryBuilder query = prefixQuery("_index", "does_not_exist"); + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class)); + } + + public void testRewriteIndexQueryToNotMatchNone() throws Exception { + PrefixQueryBuilder query = prefixQuery("_index", getIndex().getName()); + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(PrefixQueryBuilder.class)); + } + } diff --git a/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java index 64aeb1bc0ed6c..d64e1fc1a978b 100644 --- a/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/TermQueryBuilderTests.java @@ -171,6 +171,20 @@ public void testParseFailsWithMultipleFields() throws IOException { assertEquals("[term] query doesn't support multiple fields, found [message1] and [message2]", e.getMessage()); } + + public void testRewriteIndexQueryToMatchNone() throws IOException { + TermQueryBuilder query = QueryBuilders.termQuery("_index", "does_not_exist"); + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class)); + } + + public void testRewriteIndexQueryToNotMatchNone() throws IOException { + TermQueryBuilder query = QueryBuilders.termQuery("_index", getIndex().getName()); + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(TermQueryBuilder.class)); + } public void testParseAndSerializeBigInteger() throws IOException { String json = "{\n" + " \"term\" : {\n" + diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java index 326cb2523afc9..bc2be5877ef26 100644 --- a/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/TermsQueryBuilderTests.java @@ -311,5 +311,19 @@ public void testConversion() { assertEquals(Arrays.asList(5, 42d), TermsQueryBuilder.convert(list)); assertEquals(Arrays.asList(5, 42d), TermsQueryBuilder.convertBack(TermsQueryBuilder.convert(list))); } -} + public void testRewriteIndexQueryToMatchNone() throws IOException { + TermsQueryBuilder query = new TermsQueryBuilder("_index", "does_not_exist", "also_does_not_exist"); + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class)); + } + + public void testRewriteIndexQueryToNotMatchNone() throws IOException { + // At least one name is good + TermsQueryBuilder query = new TermsQueryBuilder("_index", "does_not_exist", getIndex().getName()); + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(TermsQueryBuilder.class)); + } +} diff --git a/server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java index e1bb682d9676e..7c55d6e79edb2 100644 --- a/server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/WildcardQueryBuilderTests.java @@ -154,4 +154,20 @@ public void testIndexWildcard() throws IOException { query = new WildcardQueryBuilder("_index", "index_" + index + "*").doToQuery(context); assertThat(query instanceof MatchNoDocsQuery, equalTo(true)); } + + public void testRewriteIndexQueryToMatchNone() throws IOException { + WildcardQueryBuilder query = new WildcardQueryBuilder("_index", "does_not_exist"); + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(MatchNoneQueryBuilder.class)); + } + + public void testRewriteIndexQueryNotMatchNone() throws IOException { + String fullIndexName = getIndex().getName(); + String firstHalfOfIndexName = fullIndexName.substring(0,fullIndexName.length()/2); + WildcardQueryBuilder query = new WildcardQueryBuilder("_index", firstHalfOfIndexName +"*"); + QueryShardContext queryShardContext = createShardContext(); + QueryBuilder rewritten = query.rewrite(queryShardContext); + assertThat(rewritten, instanceOf(WildcardQueryBuilder.class)); + } }