Skip to content

Commit

Permalink
Make sure to rewrite explain query on coordinator (#87013)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jtibshirani committed May 24, 2022
1 parent e64a62b commit 14ff202
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 2 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/87013.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 87013
summary: Make sure to rewrite explain query on coordinator
area: Search
type: bug
issues:
- 64281
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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");

Expand Down Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -77,7 +80,14 @@ public TransportExplainAction(
@Override
protected void doExecute(Task task, ExplainRequest request, ActionListener<ExplainResponse> listener) {
request.nowInMillis = System.currentTimeMillis();
super.doExecute(task, request, listener);
ActionListener<QueryBuilder> 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
Expand Down

0 comments on commit 14ff202

Please sign in to comment.