Skip to content

Commit c2f41cf

Browse files
girishjeyakumarGirish Jeyakumarmch2
authored
Include named queries from rescore contexts in matched_queries array (#18697)
* Fix: Include named queries from rescore contexts in matched_queries array - Modified QueryRescoreContext to store ParsedQuery instead of just Query - Updated QueryRescorerBuilder to use context.toQuery() for capturing named queries - Enhanced MatchedQueriesPhase to collect named queries from all rescore contexts - Added comprehensive tests for both unit and REST API scenarios - Resolves inconsistency with Elasticsearch behavior where rescore named queries were not surfaced - Cleaned up unnecessary comments for better code readability Signed-off-by: Girish Jeyakumar <[email protected]> * Address maintainer feedback: update RescoreContext interface design - Added parsedQuery() method to RescoreContext interface - Added getParsedQueries() method to RescoreContext interface - Updated QueryRescoreContext to override interface methods - Removed instanceof check in MatchedQueriesPhase by using interface method - Simplified assignment in QueryRescorerBuilder to one line Signed-off-by: Girish Jeyakumar <[email protected]> * Complete interface replacement: replace query() with parsedQuery() and getQueries() with getParsedQueries() - Removed query() method from QueryRescoreContext - Removed getQueries() method from RescoreContext interface - Updated all callers to use parsedQuery().query() instead of query() - Updated DfsPhase to use getParsedQueries() instead of getQueries() - Updated QueryRescorer to use parsedQuery().query() for rescoring and explanations - Updated QueryRescorerBuilderTests to use parsedQuery().query() - This addresses the maintainer's feedback for a cleaner interface design Signed-off-by: Girish Jeyakumar <[email protected]> * Finalize interface design and move changelog entry to Added section - Removed parsedQuery() method from RescoreContext base interface - Renamed getParsedQuery() to parsedQuery() in QueryRescoreContext for consistency - Updated all callers to use parsedQuery() method - Moved changelog entry to 'Added' section (more appropriate than 'Fixed') - Added proper imports for ParsedQuery to improve code readability - All tests pass and code formatting is correct Final interface design: - RescoreContext: only getParsedQueries() (returns List<ParsedQuery>) - QueryRescoreContext: parsedQuery() + getParsedQueries() override - MatchedQueriesPhase: uses getParsedQueries().forEach() for clean iteration This addresses all maintainer feedback from @mch2 and provides a clean, well-tested implementation that surfaces named queries from rescore contexts in the matched_queries array. Signed-off-by: Girish Jeyakumar <[email protected]> * Maintain backward compatibility for RescoreContext public API - Restore getQueries() method for backward compatibility - Add getParsedQueries() method for new functionality Signed-off-by: Girish Jeyakumar <[email protected]> * Fix mixed cluster test failure by updating skip version to 3.2.0 Signed-off-by: girish jeyakumar <[email protected]> --------- Signed-off-by: Girish Jeyakumar <[email protected]> Signed-off-by: girish jeyakumar <[email protected]> Signed-off-by: Marc Handalian <[email protected]> Co-authored-by: Girish Jeyakumar <[email protected]> Co-authored-by: Marc Handalian <[email protected]>
1 parent e1fc79e commit c2f41cf

File tree

8 files changed

+180
-14
lines changed

8 files changed

+180
-14
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2424
- Extend Approximation Framework to other numeric types ([#18530](https://github.com/opensearch-project/OpenSearch/issues/18530))
2525
- Add Semantic Version field type mapper and extensive unit tests([#18454](https://github.com/opensearch-project/OpenSearch/pull/18454))
2626
- Pass index settings to system ingest processor factories. ([#18708](https://github.com/opensearch-project/OpenSearch/pull/18708))
27+
- Include named queries from rescore contexts in matched_queries array ([#18697](https://github.com/opensearch-project/OpenSearch/pull/18697))
2728

2829
### Changed
2930
- Update Subject interface to use CheckedRunnable ([#18570](https://github.com/opensearch-project/OpenSearch/issues/18570))

rest-api-spec/src/main/resources/rest-api-spec/test/search/350_matched_queries.yml

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,101 @@ setup:
101101
- match: { hits.hits.0.matched_queries.match_field_2: 10 }
102102
- length: { hits.hits.1.matched_queries: 1 }
103103
- match: { hits.hits.1.matched_queries.match_field_1: 1 }
104+
105+
---
106+
107+
"named queries in rescore":
108+
- skip:
109+
version: " - 3.1.99"
110+
reason: "named queries in rescore is supported in 3.2.0 and above"
111+
112+
- do:
113+
indices.create:
114+
index: test
115+
116+
- do:
117+
bulk:
118+
refresh: true
119+
body:
120+
- '{ "index" : { "_index" : "test_1", "_id" : "1" } }'
121+
- '{"field" : 1, "title": "hello world" }'
122+
- '{ "index" : { "_index" : "test_1", "_id" : "2" } }'
123+
- '{"field" : 2, "title": "hello universe" }'
124+
125+
- do:
126+
search:
127+
index: test_1
128+
body:
129+
query:
130+
match: {
131+
field: {
132+
query: 1,
133+
_name: main_query
134+
}
135+
}
136+
rescore:
137+
window_size: 10
138+
query:
139+
rescore_query:
140+
match: {
141+
title: {
142+
query: "hello",
143+
_name: rescore_query
144+
}
145+
}
146+
query_weight: 0.5
147+
rescore_query_weight: 1.5
148+
149+
- match: { hits.total.value: 1 }
150+
- length: { hits.hits.0.matched_queries: 2 }
151+
- match: { hits.hits.0.matched_queries: [ "main_query", "rescore_query" ] }
152+
153+
---
154+
155+
"named queries in rescore with scores":
156+
- skip:
157+
version: " - 3.1.99"
158+
reason: "named queries in rescore is supported in 3.2.0 and above"
159+
160+
- do:
161+
indices.create:
162+
index: test
163+
164+
- do:
165+
bulk:
166+
refresh: true
167+
body:
168+
- '{ "index" : { "_index" : "test_1", "_id" : "1" } }'
169+
- '{"field" : 1, "title": "hello world" }'
170+
- '{ "index" : { "_index" : "test_1", "_id" : "2" } }'
171+
- '{"field" : 2, "title": "hello universe" }'
172+
173+
- do:
174+
search:
175+
include_named_queries_score: true
176+
index: test_1
177+
body:
178+
query:
179+
match: {
180+
field: {
181+
query: 1,
182+
_name: main_query
183+
}
184+
}
185+
rescore:
186+
window_size: 10
187+
query:
188+
rescore_query:
189+
match: {
190+
title: {
191+
query: "hello",
192+
_name: rescore_query
193+
}
194+
}
195+
query_weight: 0.5
196+
rescore_query_weight: 1.5
197+
198+
- match: { hits.total.value: 1 }
199+
- length: { hits.hits.0.matched_queries: 2 }
200+
- gte: { hits.hits.0.matched_queries.main_query: 0.0 }
201+
- gte: { hits.hits.0.matched_queries.rescore_query: 0.0 }

server/src/main/java/org/opensearch/search/dfs/DfsPhase.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@
3535
import org.apache.lucene.index.Term;
3636
import org.apache.lucene.search.CollectionStatistics;
3737
import org.apache.lucene.search.IndexSearcher;
38-
import org.apache.lucene.search.Query;
3938
import org.apache.lucene.search.ScoreMode;
4039
import org.apache.lucene.search.TermStatistics;
4140
import org.opensearch.core.tasks.TaskCancelledException;
41+
import org.opensearch.index.query.ParsedQuery;
4242
import org.opensearch.search.internal.SearchContext;
4343
import org.opensearch.search.rescore.RescoreContext;
4444

@@ -86,8 +86,8 @@ public CollectionStatistics collectionStatistics(String field) throws IOExceptio
8686

8787
searcher.createWeight(context.searcher().rewrite(context.query()), ScoreMode.COMPLETE, 1);
8888
for (RescoreContext rescoreContext : context.rescore()) {
89-
for (Query query : rescoreContext.getQueries()) {
90-
searcher.createWeight(context.searcher().rewrite(query), ScoreMode.COMPLETE, 1);
89+
for (ParsedQuery parsedQuery : rescoreContext.getParsedQueries()) {
90+
searcher.createWeight(context.searcher().rewrite(parsedQuery.query()), ScoreMode.COMPLETE, 1);
9191
}
9292
}
9393

server/src/main/java/org/opensearch/search/fetch/subphase/MatchedQueriesPhase.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.opensearch.search.fetch.FetchContext;
4242
import org.opensearch.search.fetch.FetchSubPhase;
4343
import org.opensearch.search.fetch.FetchSubPhaseProcessor;
44+
import org.opensearch.search.rescore.RescoreContext;
4445

4546
import java.io.IOException;
4647
import java.util.ArrayList;
@@ -65,6 +66,12 @@ public FetchSubPhaseProcessor getProcessor(FetchContext context) throws IOExcept
6566
if (context.parsedPostFilter() != null) {
6667
namedQueries.putAll(context.parsedPostFilter().namedFilters());
6768
}
69+
if (context.rescore() != null) {
70+
for (RescoreContext rescoreContext : context.rescore()) {
71+
rescoreContext.getParsedQueries().forEach(query -> namedQueries.putAll(query.namedFilters()));
72+
}
73+
}
74+
6875
if (namedQueries.isEmpty()) {
6976
return null;
7077
}

server/src/main/java/org/opensearch/search/rescore/QueryRescorer.java

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import org.apache.lucene.search.Query;
3838
import org.apache.lucene.search.ScoreDoc;
3939
import org.apache.lucene.search.TopDocs;
40+
import org.opensearch.index.query.ParsedQuery;
41+
import org.opensearch.search.rescore.QueryRescorer.QueryRescoreContext;
4042

4143
import java.io.IOException;
4244
import java.util.Arrays;
@@ -67,7 +69,7 @@ public TopDocs rescore(TopDocs topDocs, IndexSearcher searcher, RescoreContext r
6769

6870
final QueryRescoreContext rescore = (QueryRescoreContext) rescoreContext;
6971

70-
org.apache.lucene.search.Rescorer rescorer = new org.apache.lucene.search.QueryRescorer(rescore.query()) {
72+
org.apache.lucene.search.Rescorer rescorer = new org.apache.lucene.search.QueryRescorer(rescore.parsedQuery().query()) {
7173

7274
@Override
7375
protected float combine(float firstPassScore, boolean secondPassMatches, float secondPassScore) {
@@ -120,7 +122,7 @@ public Explanation explain(int topLevelDocId, IndexSearcher searcher, RescoreCon
120122
prim = Explanation.noMatch("First pass did not match", sourceExplanation);
121123
}
122124
if (rescoreContext.isRescored(topLevelDocId)) {
123-
Explanation rescoreExplain = searcher.explain(rescore.query(), topLevelDocId);
125+
Explanation rescoreExplain = searcher.explain(rescore.parsedQuery().query(), topLevelDocId);
124126
// NOTE: we don't use Lucene's Rescorer.explain because we want to insert our own description with which ScoreMode was used.
125127
// Maybe we should add QueryRescorer.explainCombine to Lucene?
126128
if (rescoreExplain != null && rescoreExplain.isMatch()) {
@@ -190,7 +192,7 @@ private TopDocs combine(TopDocs in, TopDocs resorted, QueryRescoreContext ctx) {
190192
* @opensearch.internal
191193
*/
192194
public static class QueryRescoreContext extends RescoreContext {
193-
private Query query;
195+
private ParsedQuery parsedQuery;
194196
private float queryWeight = 1.0f;
195197
private float rescoreQueryWeight = 1.0f;
196198
private QueryRescoreMode scoreMode;
@@ -200,17 +202,22 @@ public QueryRescoreContext(int windowSize) {
200202
this.scoreMode = QueryRescoreMode.Total;
201203
}
202204

203-
public void setQuery(Query query) {
204-
this.query = query;
205+
public void setParsedQuery(ParsedQuery parsedQuery) {
206+
this.parsedQuery = parsedQuery;
207+
}
208+
209+
public ParsedQuery parsedQuery() {
210+
return parsedQuery;
205211
}
206212

207213
@Override
208214
public List<Query> getQueries() {
209-
return Collections.singletonList(query);
215+
return parsedQuery != null ? Collections.singletonList(parsedQuery.query()) : Collections.emptyList();
210216
}
211217

212-
public Query query() {
213-
return query;
218+
@Override
219+
public List<ParsedQuery> getParsedQueries() {
220+
return parsedQuery != null ? Collections.singletonList(parsedQuery) : Collections.emptyList();
214221
}
215222

216223
public float queryWeight() {

server/src/main/java/org/opensearch/search/rescore/QueryRescorerBuilder.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,9 @@ public static QueryRescorerBuilder fromXContent(XContentParser parser) throws IO
190190
@Override
191191
public QueryRescoreContext innerBuildContext(int windowSize, QueryShardContext context) throws IOException {
192192
QueryRescoreContext queryRescoreContext = new QueryRescoreContext(windowSize);
193-
// query is rewritten at this point already
194-
queryRescoreContext.setQuery(queryBuilder.toQuery(context));
193+
194+
queryRescoreContext.setParsedQuery(context.toQuery(queryBuilder));
195+
195196
queryRescoreContext.setQueryWeight(this.queryWeight);
196197
queryRescoreContext.setRescoreQueryWeight(this.rescoreQueryWeight);
197198
queryRescoreContext.setScoreMode(this.scoreMode);

server/src/main/java/org/opensearch/search/rescore/RescoreContext.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434

3535
import org.apache.lucene.search.Query;
3636
import org.opensearch.common.annotation.PublicApi;
37+
import org.opensearch.index.query.ParsedQuery;
3738

3839
import java.util.Collections;
3940
import java.util.List;
@@ -93,4 +94,11 @@ public Set<Integer> getRescoredDocs() {
9394
public List<Query> getQueries() {
9495
return Collections.emptyList();
9596
}
97+
98+
/**
99+
* Returns parsed queries associated with the rescorer
100+
*/
101+
public List<ParsedQuery> getParsedQueries() {
102+
return Collections.emptyList();
103+
}
96104
}

server/src/test/java/org/opensearch/search/rescore/QueryRescorerBuilderTests.java

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ public MappedFieldType fieldMapper(String name) {
190190
: rescoreBuilder.windowSize().intValue();
191191
assertEquals(expectedWindowSize, rescoreContext.getWindowSize());
192192
Query expectedQuery = Rewriteable.rewrite(rescoreBuilder.getRescoreQuery(), mockShardContext).toQuery(mockShardContext);
193-
assertEquals(expectedQuery, rescoreContext.query());
193+
assertEquals(expectedQuery, rescoreContext.parsedQuery().query());
194194
assertEquals(rescoreBuilder.getQueryWeight(), rescoreContext.queryWeight(), Float.MIN_VALUE);
195195
assertEquals(rescoreBuilder.getRescoreQueryWeight(), rescoreContext.rescoreQueryWeight(), Float.MIN_VALUE);
196196
assertEquals(rescoreBuilder.getScoreMode(), rescoreContext.scoreMode());
@@ -202,6 +202,50 @@ public void testRescoreQueryNull() throws IOException {
202202
assertEquals("rescore_query cannot be null", e.getMessage());
203203
}
204204

205+
/**
206+
* Test that named queries from rescore contexts are captured
207+
*/
208+
public void testRescoreNamedQueries() throws IOException {
209+
final long nowInMillis = randomNonNegativeLong();
210+
Settings indexSettings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT).build();
211+
IndexSettings idxSettings = IndexSettingsModule.newIndexSettings(randomAlphaOfLengthBetween(1, 10), indexSettings);
212+
213+
QueryShardContext mockShardContext = new QueryShardContext(
214+
0,
215+
idxSettings,
216+
BigArrays.NON_RECYCLING_INSTANCE,
217+
null,
218+
null,
219+
null,
220+
null,
221+
null,
222+
xContentRegistry(),
223+
namedWriteableRegistry,
224+
null,
225+
null,
226+
() -> nowInMillis,
227+
null,
228+
null,
229+
() -> true,
230+
null
231+
) {
232+
@Override
233+
public MappedFieldType fieldMapper(String name) {
234+
TextFieldMapper.Builder builder = new TextFieldMapper.Builder(name, createDefaultIndexAnalyzers());
235+
return builder.build(new Mapper.BuilderContext(idxSettings.getSettings(), new ContentPath(1))).fieldType();
236+
}
237+
};
238+
239+
QueryBuilder namedQueryBuilder = new MatchAllQueryBuilder().queryName("test_rescore_query");
240+
QueryRescorerBuilder rescoreBuilder = new QueryRescorerBuilder(namedQueryBuilder);
241+
QueryRescoreContext rescoreContext = (QueryRescoreContext) rescoreBuilder.buildContext(mockShardContext);
242+
assertNotNull(rescoreContext.parsedQuery());
243+
assertNotNull(rescoreContext.parsedQuery().namedFilters());
244+
assertEquals(1, rescoreContext.parsedQuery().namedFilters().size());
245+
assertTrue(rescoreContext.parsedQuery().namedFilters().containsKey("test_rescore_query"));
246+
assertNotNull(rescoreContext.parsedQuery().namedFilters().get("test_rescore_query"));
247+
}
248+
205249
class AlwaysRewriteQueryBuilder extends MatchAllQueryBuilder {
206250

207251
@Override

0 commit comments

Comments
 (0)