Named queries should return their internal score as well

While some internal queries may not be scored or not contrbute to the final doc score, in some cases (such as knn queries compbined with other queries) the distance metric is very interesting and should be reported back as part of the response. This change adds that by default, so named queries always return the score (instead of just the name). It adds no additional cost. Signed-off-by: Itamar Syn-Hershko <[email protected]>
opensearch-project · Jun 4, 2023 · e715622 · e715622
1 parent 5d5e8ad
commit e715622
Show file tree

Hide file tree

Showing 11 changed files with 189 additions and 122 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Allow mmap to use new JDK-19 preview APIs in Apache Lucene 9.4+ ([#5151](https://github.com/opensearch-project/OpenSearch/pull/5151))
 - Add events correlation engine plugin ([#6854](https://github.com/opensearch-project/OpenSearch/issues/6854))
 - Add support for ignoring missing Javadoc on generated code using annotation ([#7604](https://github.com/opensearch-project/OpenSearch/pull/7604))
+- Named queries now return their internal score as well ([#7883](https://github.com/opensearch-project/OpenSearch/pull/7883))
 
 ### Dependencies
 - Bump `log4j-core` from 2.18.0 to 2.19.0

diff --git a/...arent-join/src/internalClusterTest/java/org/opensearch/join/query/ChildQuerySearchIT.java b/...arent-join/src/internalClusterTest/java/org/opensearch/join/query/ChildQuerySearchIT.java
@@ -72,6 +72,11 @@
 import java.util.Map;
 import java.util.Set;
 
+import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasKey;
+import static org.hamcrest.Matchers.is;
 import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.opensearch.common.xcontent.support.XContentMapValues.extractValue;
 import static org.opensearch.index.query.QueryBuilders.boolQuery;
@@ -93,10 +98,6 @@
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchHit;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchHits;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.hasId;
-import static org.hamcrest.Matchers.anyOf;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.is;
 
 public class ChildQuerySearchIT extends ParentChildTestCase {
 
@@ -1250,29 +1251,29 @@ public void testNamedFilters() throws Exception {
             .setQuery(hasChildQuery("child", termQuery("c_field", "1"), ScoreMode.Max).queryName("test"))
             .get();
         assertHitCount(searchResponse, 1L);
-        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries().length, equalTo(1));
-        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries()[0], equalTo("test"));
+        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries().size(), equalTo(1));
+        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries(), hasKey("test"));
 
         searchResponse = client().prepareSearch("test")
             .setQuery(hasParentQuery("parent", termQuery("p_field", "1"), true).queryName("test"))
             .get();
         assertHitCount(searchResponse, 1L);
-        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries().length, equalTo(1));
-        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries()[0], equalTo("test"));
+        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries().size(), equalTo(1));
+        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries(), hasKey("test"));
 
         searchResponse = client().prepareSearch("test")
             .setQuery(constantScoreQuery(hasChildQuery("child", termQuery("c_field", "1"), ScoreMode.None).queryName("test")))
             .get();
         assertHitCount(searchResponse, 1L);
-        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries().length, equalTo(1));
-        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries()[0], equalTo("test"));
+        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries().size(), equalTo(1));
+        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries(), hasKey("test"));
 
         searchResponse = client().prepareSearch("test")
             .setQuery(constantScoreQuery(hasParentQuery("parent", termQuery("p_field", "1"), false).queryName("test")))
             .get();
         assertHitCount(searchResponse, 1L);
-        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries().length, equalTo(1));
-        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries()[0], equalTo("test"));
+        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries().size(), equalTo(1));
+        assertThat(searchResponse.getHits().getAt(0).getMatchedQueries(), hasKey("test"));
     }
 
     public void testParentChildQueriesNoParentType() throws Exception {

diff --git a/modules/parent-join/src/internalClusterTest/java/org/opensearch/join/query/InnerHitsIT.java b/modules/parent-join/src/internalClusterTest/java/org/opensearch/join/query/InnerHitsIT.java
@@ -61,6 +61,12 @@
 import java.util.Map;
 import java.util.function.Function;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.hasKey;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
 import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.opensearch.common.xcontent.support.XContentMapValues.extractValue;
 import static org.opensearch.index.query.QueryBuilders.boolQuery;
@@ -79,11 +85,6 @@
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchHit;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchHits;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.hasId;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.greaterThanOrEqualTo;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.nullValue;
 
 public class InnerHitsIT extends ParentChildTestCase {
 
@@ -545,13 +546,13 @@ public void testMatchesQueriesParentChildInnerHits() throws Exception {
         assertHitCount(response, 2);
         assertThat(response.getHits().getAt(0).getId(), equalTo("1"));
         assertThat(response.getHits().getAt(0).getInnerHits().get("child").getTotalHits().value, equalTo(1L));
-        assertThat(response.getHits().getAt(0).getInnerHits().get("child").getAt(0).getMatchedQueries().length, equalTo(1));
-        assertThat(response.getHits().getAt(0).getInnerHits().get("child").getAt(0).getMatchedQueries()[0], equalTo("_name1"));
+        assertThat(response.getHits().getAt(0).getInnerHits().get("child").getAt(0).getMatchedQueries().size(), equalTo(1));
+        assertThat(response.getHits().getAt(0).getInnerHits().get("child").getAt(0).getMatchedQueries(), hasKey("_name1"));
 
         assertThat(response.getHits().getAt(1).getId(), equalTo("2"));
         assertThat(response.getHits().getAt(1).getInnerHits().get("child").getTotalHits().value, equalTo(1L));
-        assertThat(response.getHits().getAt(1).getInnerHits().get("child").getAt(0).getMatchedQueries().length, equalTo(1));
-        assertThat(response.getHits().getAt(1).getInnerHits().get("child").getAt(0).getMatchedQueries()[0], equalTo("_name1"));
+        assertThat(response.getHits().getAt(1).getInnerHits().get("child").getAt(0).getMatchedQueries().size(), equalTo(1));
+        assertThat(response.getHits().getAt(1).getInnerHits().get("child").getAt(0).getMatchedQueries(), hasKey("_name1"));
 
         QueryBuilder query = hasChildQuery("child", matchQuery("field", "value2").queryName("_name2"), ScoreMode.None).innerHit(
             new InnerHitBuilder()
@@ -560,8 +561,8 @@ public void testMatchesQueriesParentChildInnerHits() throws Exception {
         assertHitCount(response, 1);
         assertThat(response.getHits().getAt(0).getId(), equalTo("1"));
         assertThat(response.getHits().getAt(0).getInnerHits().get("child").getTotalHits().value, equalTo(1L));
-        assertThat(response.getHits().getAt(0).getInnerHits().get("child").getAt(0).getMatchedQueries().length, equalTo(1));
-        assertThat(response.getHits().getAt(0).getInnerHits().get("child").getAt(0).getMatchedQueries()[0], equalTo("_name2"));
+        assertThat(response.getHits().getAt(0).getInnerHits().get("child").getAt(0).getMatchedQueries().size(), equalTo(1));
+        assertThat(response.getHits().getAt(0).getInnerHits().get("child").getAt(0).getMatchedQueries(), hasKey("_name2"));
     }
 
     public void testUseMaxDocInsteadOfSize() throws Exception {

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/340_named_queries.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/340_named_queries.yml
@@ -0,0 +1,55 @@
+---
+"Named queries with score":
+  - do:
+      indices.create:
+        index:  test
+  - do:
+      index:
+        index:  test
+        id:     1
+        body:   { foo: bar }
+
+  - do:
+      index:
+        index:  test
+        id:     2
+        body:   { foo: bar, foo2: bar2 }
+
+  - do:
+      indices.refresh:
+        index: [test]
+
+  - do:
+      search:
+        index: test
+        body:
+          query:
+            bool: {
+              should: [
+                {
+                  match: {
+                    foo: {
+                      query: bar,
+                      _name: foo_query,
+                      boost: 5
+                    }
+                  }
+                },
+                {
+                  match: {
+                    foo2: {
+                      query: bar2,
+                      _name: foo2_query,
+                      boost: 20
+                    }
+                  }
+                }
+              ]
+            }
+
+  - match:  {hits.total.value: 2}
+  - length: {hits.hits.0.matched_queries: 2}
+  - gt: {hits.hits.0.matched_queries.foo_query: 0.0}
+  - gt: {hits.hits.0.matched_queries.foo2_query: 0.0}
+  - length: {hits.hits.1.matched_queries: 1 }
+  - gt: {hits.hits.1.matched_queries.foo_query: 0.0}
diff --git a/...er/src/internalClusterTest/java/org/opensearch/search/aggregations/metrics/TopHitsIT.java b/...er/src/internalClusterTest/java/org/opensearch/search/aggregations/metrics/TopHitsIT.java
@@ -76,6 +76,16 @@
 import java.util.Map;
 import java.util.function.Function;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.hasKey;
+import static org.hamcrest.Matchers.lessThanOrEqualTo;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+import static org.hamcrest.Matchers.sameInstance;
 import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.opensearch.common.xcontent.XContentFactory.smileBuilder;
 import static org.opensearch.common.xcontent.XContentFactory.yamlBuilder;
@@ -93,16 +103,6 @@
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertHitCount;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertNoFailures;
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertSearchResponse;
-import static org.hamcrest.Matchers.arrayContaining;
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.greaterThanOrEqualTo;
-import static org.hamcrest.Matchers.lessThanOrEqualTo;
-import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.nullValue;
-import static org.hamcrest.Matchers.sameInstance;
 
 @OpenSearchIntegTestCase.SuiteScopeTestCase()
 public class TopHitsIT extends OpenSearchIntegTestCase {
@@ -686,7 +686,7 @@ public void testFetchFeatures() {
                 assertThat(hit.getPrimaryTerm(), equalTo(SequenceNumbers.UNASSIGNED_PRIMARY_TERM));
             }
 
-            assertThat(hit.getMatchedQueries()[0], equalTo("test"));
+            assertThat(hit.getMatchedQueries(), hasKey("test"));
 
             DocumentField field1 = hit.field("field1");
             assertThat(field1.getValue(), equalTo(5L));
@@ -961,7 +961,7 @@ public void testNestedFetchFeatures() {
         long version = searchHit.getVersion();
         assertThat(version, equalTo(1L));
 
-        assertThat(searchHit.getMatchedQueries(), arrayContaining("test"));
+        assertThat(searchHit.getMatchedQueries(), hasKey("test"));
 
         DocumentField field = searchHit.field("comments.user");
         assertThat(field.getValue().toString(), equalTo("a"));

diff --git a/server/src/internalClusterTest/java/org/opensearch/search/fetch/subphase/InnerHitsIT.java b/server/src/internalClusterTest/java/org/opensearch/search/fetch/subphase/InnerHitsIT.java
@@ -81,6 +81,7 @@
 import static org.opensearch.test.hamcrest.OpenSearchAssertions.hasId;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
@@ -833,21 +834,21 @@ public void testMatchesQueriesNestedInnerHits() throws Exception {
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo((long) numDocs));
         assertThat(searchResponse.getHits().getAt(0).getId(), equalTo("0"));
         assertThat(searchResponse.getHits().getAt(0).getInnerHits().get("nested1").getTotalHits().value, equalTo(2L));
-        assertThat(searchResponse.getHits().getAt(0).getInnerHits().get("nested1").getAt(0).getMatchedQueries().length, equalTo(1));
-        assertThat(searchResponse.getHits().getAt(0).getInnerHits().get("nested1").getAt(0).getMatchedQueries()[0], equalTo("test1"));
-        assertThat(searchResponse.getHits().getAt(0).getInnerHits().get("nested1").getAt(1).getMatchedQueries().length, equalTo(1));
-        assertThat(searchResponse.getHits().getAt(0).getInnerHits().get("nested1").getAt(1).getMatchedQueries()[0], equalTo("test3"));
+        assertThat(searchResponse.getHits().getAt(0).getInnerHits().get("nested1").getAt(0).getMatchedQueries().size(), equalTo(1));
+        assertThat(searchResponse.getHits().getAt(0).getInnerHits().get("nested1").getAt(0).getMatchedQueries(), hasKey("test1"));
+        assertThat(searchResponse.getHits().getAt(0).getInnerHits().get("nested1").getAt(1).getMatchedQueries().size(), equalTo(1));
+        assertThat(searchResponse.getHits().getAt(0).getInnerHits().get("nested1").getAt(1).getMatchedQueries(), hasKey("test3"));
 
         assertThat(searchResponse.getHits().getAt(1).getId(), equalTo("1"));
         assertThat(searchResponse.getHits().getAt(1).getInnerHits().get("nested1").getTotalHits().value, equalTo(1L));
-        assertThat(searchResponse.getHits().getAt(1).getInnerHits().get("nested1").getAt(0).getMatchedQueries().length, equalTo(1));
-        assertThat(searchResponse.getHits().getAt(1).getInnerHits().get("nested1").getAt(0).getMatchedQueries()[0], equalTo("test2"));
+        assertThat(searchResponse.getHits().getAt(1).getInnerHits().get("nested1").getAt(0).getMatchedQueries().size(), equalTo(1));
+        assertThat(searchResponse.getHits().getAt(1).getInnerHits().get("nested1").getAt(0).getMatchedQueries(), hasKey("test2"));
 
         for (int i = 2; i < numDocs; i++) {
             assertThat(searchResponse.getHits().getAt(i).getId(), equalTo(String.valueOf(i)));
             assertThat(searchResponse.getHits().getAt(i).getInnerHits().get("nested1").getTotalHits().value, equalTo(1L));
-            assertThat(searchResponse.getHits().getAt(i).getInnerHits().get("nested1").getAt(0).getMatchedQueries().length, equalTo(1));
-            assertThat(searchResponse.getHits().getAt(i).getInnerHits().get("nested1").getAt(0).getMatchedQueries()[0], equalTo("test3"));
+            assertThat(searchResponse.getHits().getAt(i).getInnerHits().get("nested1").getAt(0).getMatchedQueries().size(), equalTo(1));
+            assertThat(searchResponse.getHits().getAt(i).getInnerHits().get("nested1").getAt(0).getMatchedQueries(), hasKey("test3"));
         }
     }