Skip to content

Commit 08e8f24

Browse files
committed
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.
1 parent 3272a53 commit 08e8f24

File tree

3 files changed

+42
-2
lines changed

3 files changed

+42
-2
lines changed

docs/changelog/87013.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 87013
2+
summary: Make sure to rewrite explain query on coordinator
3+
area: Search
4+
type: bug
5+
issues:
6+
- 64281

server/src/internalClusterTest/java/org/elasticsearch/explain/ExplainActionIT.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import org.elasticsearch.common.lucene.Lucene;
1717
import org.elasticsearch.common.settings.Settings;
1818
import org.elasticsearch.index.query.QueryBuilders;
19+
import org.elasticsearch.index.query.TermsQueryBuilder;
20+
import org.elasticsearch.indices.TermsLookup;
1921
import org.elasticsearch.test.ESIntegTestCase;
2022

2123
import java.io.ByteArrayInputStream;
@@ -28,6 +30,7 @@
2830
import java.util.Set;
2931

3032
import static java.util.Collections.singleton;
33+
import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS;
3134
import static org.elasticsearch.index.query.QueryBuilders.queryStringQuery;
3235
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
3336
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
@@ -151,7 +154,7 @@ public void testExplainWithFields() throws Exception {
151154
}
152155

153156
@SuppressWarnings("unchecked")
154-
public void testExplainWitSource() throws Exception {
157+
public void testExplainWithSource() throws Exception {
155158
assertAcked(prepareCreate("test").addAlias(new Alias("alias")));
156159
ensureGreen("test");
157160

@@ -281,4 +284,25 @@ public void testStreamExplain() throws Exception {
281284
result = Lucene.readExplanation(esBuffer);
282285
assertThat(exp.toString(), equalTo(result.toString()));
283286
}
287+
288+
public void testQueryRewrite() {
289+
client().admin()
290+
.indices()
291+
.prepareCreate("twitter")
292+
.setMapping("user", "type=keyword", "followers", "type=keyword")
293+
.setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 2))
294+
.get();
295+
ensureGreen("twitter");
296+
297+
client().prepareIndex("twitter").setId("1").setSource("user", "user1", "followers", new String[] { "user2", "user3" }).get();
298+
client().prepareIndex("twitter").setId("2").setSource("user", "user2", "followers", new String[] { "user1" }).get();
299+
refresh();
300+
301+
TermsQueryBuilder termsLookupQuery = QueryBuilders.termsLookupQuery("user", new TermsLookup("twitter", "2", "followers"));
302+
ExplainResponse response = client().prepareExplain("twitter", "1").setQuery(termsLookupQuery).execute().actionGet();
303+
304+
Explanation explanation = response.getExplanation();
305+
assertNotNull(explanation);
306+
assertTrue(explanation.isMatch());
307+
}
284308
}

server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import org.elasticsearch.index.IndexService;
2424
import org.elasticsearch.index.engine.Engine;
2525
import org.elasticsearch.index.get.GetResult;
26+
import org.elasticsearch.index.query.QueryBuilder;
27+
import org.elasticsearch.index.query.Rewriteable;
2628
import org.elasticsearch.index.shard.IndexShard;
2729
import org.elasticsearch.index.shard.ShardId;
2830
import org.elasticsearch.search.SearchService;
@@ -37,6 +39,7 @@
3739

3840
import java.io.IOException;
3941
import java.util.Set;
42+
import java.util.function.LongSupplier;
4043

4144
/**
4245
* Explain transport action. Computes the explain on the targeted shard.
@@ -71,7 +74,14 @@ public TransportExplainAction(
7174
@Override
7275
protected void doExecute(Task task, ExplainRequest request, ActionListener<ExplainResponse> listener) {
7376
request.nowInMillis = System.currentTimeMillis();
74-
super.doExecute(task, request, listener);
77+
ActionListener<QueryBuilder> rewriteListener = ActionListener.wrap(rewrittenQuery -> {
78+
request.query(rewrittenQuery);
79+
super.doExecute(task, request, listener);
80+
}, listener::onFailure);
81+
82+
assert request.query() != null;
83+
LongSupplier timeProvider = () -> request.nowInMillis;
84+
Rewriteable.rewriteAndFetch(request.query(), searchService.getRewriteContext(timeProvider), rewriteListener);
7585
}
7686

7787
@Override

0 commit comments

Comments
 (0)