Browse Source

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.
Julie Tibshirani 3 years ago
parent
commit
88fc893e8d

+ 6 - 0
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

+ 25 - 1
server/src/internalClusterTest/java/org/elasticsearch/explain/ExplainActionIT.java

@@ -16,6 +16,8 @@ import org.elasticsearch.common.io.stream.OutputStreamStreamOutput;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.settings.Settings;
 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 +30,7 @@ import java.util.Map;
 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;
@@ -151,7 +154,7 @@ public class ExplainActionIT extends ESIntegTestCase {
     }
 
     @SuppressWarnings("unchecked")
-    public void testExplainWitSource() throws Exception {
+    public void testExplainWithSource() throws Exception {
         assertAcked(prepareCreate("test").addAlias(new Alias("alias")));
         ensureGreen("test");
 
@@ -281,4 +284,25 @@ public class ExplainActionIT extends ESIntegTestCase {
         result = Lucene.readExplanation(esBuffer);
         assertThat(exp.toString(), equalTo(result.toString()));
     }
+
+    public void testQueryRewrite() {
+        client().admin()
+            .indices()
+            .prepareCreate("twitter")
+            .setMapping("user", "type=keyword", "followers", "type=keyword")
+            .setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 2))
+            .get();
+        ensureGreen("twitter");
+
+        client().prepareIndex("twitter").setId("1").setSource("user", "user1", "followers", new String[] { "user2", "user3" }).get();
+        client().prepareIndex("twitter").setId("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", "1").setQuery(termsLookupQuery).execute().actionGet();
+
+        Explanation explanation = response.getExplanation();
+        assertNotNull(explanation);
+        assertTrue(explanation.isMatch());
+    }
 }

+ 11 - 1
server/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java

@@ -23,6 +23,8 @@ import org.elasticsearch.core.Releasables;
 import org.elasticsearch.index.IndexService;
 import org.elasticsearch.index.engine.Engine;
 import org.elasticsearch.index.get.GetResult;
+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;
@@ -37,6 +39,7 @@ import org.elasticsearch.transport.TransportService;
 
 import java.io.IOException;
 import java.util.Set;
+import java.util.function.LongSupplier;
 
 /**
  * Explain transport action. Computes the explain on the targeted shard.
@@ -71,7 +74,14 @@ public class TransportExplainAction extends TransportSingleShardAction<ExplainRe
     @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