From 14ff2024be19df02d0318542ab4af3310bd25555 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 24 May 2022 13:04:14 -0700 Subject: [PATCH] Make sure to rewrite explain query on coordinator (#87013) Some queries (like terms lookup queries) need to be rewritten on the coordinator node, usually to fetch some resource. The explain action was missing this rewrite step, which caused failures later when trying to execute the query. Closes #64281. --- docs/changelog/87013.yaml | 6 ++++ .../explain/ExplainActionIT.java | 33 ++++++++++++++++++- .../explain/TransportExplainAction.java | 12 ++++++- 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 docs/changelog/87013.yaml diff --git a/docs/changelog/87013.yaml b/docs/changelog/87013.yaml new file mode 100644 index 0000000000000..539258194d0bc --- /dev/null +++ b/docs/changelog/87013.yaml @@ -0,0 +1,6 @@ +pr: 87013 +summary: Make sure to rewrite explain query on coordinator +area: Search +type: bug +issues: + - 64281 diff --git a/server/src/internalClusterTest/java/org/elasticsearch/explain/ExplainActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/explain/ExplainActionIT.java index 1b9532e25e47b..1ca86144dd43a 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/explain/ExplainActionIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/explain/ExplainActionIT.java @@ -15,7 +15,10 @@ import org.elasticsearch.common.io.stream.OutputStreamStreamOutput; import org.elasticsearch.common.lucene.Lucene; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.index.query.TermsQueryBuilder; +import org.elasticsearch.indices.TermsLookup; import org.elasticsearch.test.ESIntegTestCase; import java.io.ByteArrayInputStream; @@ -28,6 +31,7 @@ import java.util.Set; import static java.util.Collections.singleton; +import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; import static org.elasticsearch.index.query.QueryBuilders.queryStringQuery; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; @@ -155,7 +159,7 @@ public void testExplainWithFields() throws Exception { } @SuppressWarnings("unchecked") - public void testExplainWitSource() throws Exception { + public void testExplainWithSource() throws Exception { assertAcked(prepareCreate("test").addAlias(new Alias("alias"))); ensureGreen("test"); @@ -288,4 +292,31 @@ public void testStreamExplain() throws Exception { result = Lucene.readExplanation(esBuffer); assertThat(exp.toString(), equalTo(result.toString())); } + + public void testQueryRewrite() { + client().admin() + .indices() + .prepareCreate("twitter") + .addMapping(MapperService.SINGLE_MAPPING_NAME, "user", "type=keyword", "followers", "type=keyword") + .setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 2)) + .get(); + ensureGreen("twitter"); + + client().prepareIndex("twitter", MapperService.SINGLE_MAPPING_NAME, "1") + .setSource("user", "user1", "followers", new String[] { "user2", "user3" }) + .get(); + client().prepareIndex("twitter", MapperService.SINGLE_MAPPING_NAME, "2") + .setSource("user", "user2", "followers", new String[] { "user1" }) + .get(); + refresh(); + + TermsQueryBuilder termsLookupQuery = QueryBuilders.termsLookupQuery("user", new TermsLookup("twitter", "2", "followers")); + ExplainResponse response = client().prepareExplain("twitter", MapperService.SINGLE_MAPPING_NAME, "1") + .setQuery(termsLookupQuery) + .get(); + + Explanation explanation = response.getExplanation(); + assertNotNull(explanation); + assertTrue(explanation.isMatch()); + } } diff --git a/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java b/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java index 9e0bf533054b4..dbd791424621b 100644 --- a/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java +++ b/server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java @@ -29,6 +29,8 @@ import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.Uid; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.Rewriteable; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.search.SearchService; @@ -43,6 +45,7 @@ import java.io.IOException; import java.util.Set; +import java.util.function.LongSupplier; /** * Explain transport action. Computes the explain on the targeted shard. @@ -77,7 +80,14 @@ public TransportExplainAction( @Override protected void doExecute(Task task, ExplainRequest request, ActionListener listener) { request.nowInMillis = System.currentTimeMillis(); - super.doExecute(task, request, listener); + ActionListener rewriteListener = ActionListener.wrap(rewrittenQuery -> { + request.query(rewrittenQuery); + super.doExecute(task, request, listener); + }, listener::onFailure); + + assert request.query() != null; + LongSupplier timeProvider = () -> request.nowInMillis; + Rewriteable.rewriteAndFetch(request.query(), searchService.getRewriteContext(timeProvider), rewriteListener); } @Override