Skip to content

Commit 0a77be1

Browse files
authored
Fix Unified highlighter for nested fields when using matchPhrasePrefixQuery (#19442)
1 parent a391ea3 commit 0a77be1

File tree

4 files changed

+57
-8
lines changed

4 files changed

+57
-8
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
4747
- Fix file-based ingestion consumer to handle start point beyond max line number([#19757])(https://github.com/opensearch-project/OpenSearch/pull/19757))
4848
- Fix IndexOutOfBoundsException when running include/exclude on non-existent prefix in terms aggregations ([#19637](https://github.com/opensearch-project/OpenSearch/pull/19637))
4949
- Fixed assertion unsafe use of ClusterService.state() in ResourceUsageCollectorService ([#19775])(https://github.com/opensearch-project/OpenSearch/pull/19775))
50+
- Fix Unified highlighter for nested fields when using matchPhrasePrefixQuery ([#19442](https://github.com/opensearch-project/OpenSearch/pull/19442))
5051
- Add S3Repository.LEGACY_MD5_CHECKSUM_CALCULATION to list of repository-s3 settings ([#19788](https://github.com/opensearch-project/OpenSearch/pull/19788))
5152

5253
### Dependencies

server/src/internalClusterTest/java/org/opensearch/search/fetch/subphase/highlight/HighlighterSearchIT.java

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@
9999
import static org.opensearch.index.query.QueryBuilders.constantScoreQuery;
100100
import static org.opensearch.index.query.QueryBuilders.existsQuery;
101101
import static org.opensearch.index.query.QueryBuilders.fuzzyQuery;
102+
import static org.opensearch.index.query.QueryBuilders.matchPhrasePrefixQuery;
102103
import static org.opensearch.index.query.QueryBuilders.matchPhraseQuery;
103104
import static org.opensearch.index.query.QueryBuilders.matchQuery;
104105
import static org.opensearch.index.query.QueryBuilders.multiMatchQuery;
@@ -3518,7 +3519,7 @@ public void testWithNestedQuery() throws Exception {
35183519
jsonBuilder().startObject()
35193520
.startArray("foo")
35203521
.startObject()
3521-
.field("text", "brown")
3522+
.field("text", "brown cat")
35223523
.endObject()
35233524
.startObject()
35243525
.field("text", "cow")
@@ -3539,7 +3540,7 @@ public void testWithNestedQuery() throws Exception {
35393540
assertHitCount(searchResponse, 1);
35403541
HighlightField field = searchResponse.getHits().getAt(0).getHighlightFields().get("foo.text");
35413542
assertThat(field.getFragments().length, equalTo(2));
3542-
assertThat(field.getFragments()[0].string(), equalTo("<em>brown</em>"));
3543+
assertThat(field.getFragments()[0].string(), equalTo("<em>brown</em> cat"));
35433544
assertThat(field.getFragments()[1].string(), equalTo("<em>cow</em>"));
35443545

35453546
searchResponse = client().prepareSearch()
@@ -3549,16 +3550,25 @@ public void testWithNestedQuery() throws Exception {
35493550
assertHitCount(searchResponse, 1);
35503551
field = searchResponse.getHits().getAt(0).getHighlightFields().get("foo.text");
35513552
assertThat(field.getFragments().length, equalTo(1));
3552-
assertThat(field.getFragments()[0].string(), equalTo("<em>brown</em>"));
3553+
assertThat(field.getFragments()[0].string(), equalTo("<em>brown</em> cat"));
35533554

35543555
searchResponse = client().prepareSearch()
3555-
.setQuery(nestedQuery("foo", prefixQuery("foo.text", "bro"), ScoreMode.None))
3556-
.highlighter(new HighlightBuilder().field(new Field("foo.text").highlighterType("plain")))
3556+
.setQuery(nestedQuery("foo", matchPhraseQuery("foo.text", "brown cat"), ScoreMode.None))
3557+
.highlighter(new HighlightBuilder().field(new Field("foo.text").highlighterType(type)))
35573558
.get();
35583559
assertHitCount(searchResponse, 1);
35593560
field = searchResponse.getHits().getAt(0).getHighlightFields().get("foo.text");
35603561
assertThat(field.getFragments().length, equalTo(1));
3561-
assertThat(field.getFragments()[0].string(), equalTo("<em>brown</em>"));
3562+
assertThat(field.getFragments()[0].string(), equalTo("<em>brown</em> <em>cat</em>"));
3563+
3564+
searchResponse = client().prepareSearch()
3565+
.setQuery(nestedQuery("foo", matchPhrasePrefixQuery("foo.text", "bro"), ScoreMode.None))
3566+
.highlighter(new HighlightBuilder().field(new Field("foo.text").highlighterType(type)))
3567+
.get();
3568+
assertHitCount(searchResponse, 1);
3569+
field = searchResponse.getHits().getAt(0).getHighlightFields().get("foo.text");
3570+
assertThat(field.getFragments().length, equalTo(1));
3571+
assertThat(field.getFragments()[0].string(), equalTo("<em>brown</em> cat"));
35623572
}
35633573

35643574
// For unified and fvh highlighters we just check that the nested query is correctly extracted

server/src/main/java/org/opensearch/lucene/search/uhighlight/CustomUnifiedHighlighter.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import org.opensearch.common.Nullable;
5353
import org.opensearch.common.lucene.search.MultiPhrasePrefixQuery;
5454
import org.opensearch.index.IndexSettings;
55+
import org.opensearch.index.search.OpenSearchToParentBlockJoinQuery;
5556

5657
import java.io.IOException;
5758
import java.text.BreakIterator;
@@ -228,8 +229,7 @@ protected Collection<Query> preSpanQueryRewrite(Query query) {
228229
* Translate custom queries in queries that are supported by the unified highlighter.
229230
*/
230231
private Collection<Query> rewriteCustomQuery(Query query) {
231-
if (query instanceof MultiPhrasePrefixQuery) {
232-
MultiPhrasePrefixQuery mpq = (MultiPhrasePrefixQuery) query;
232+
if (query instanceof MultiPhrasePrefixQuery mpq) {
233233
Term[][] terms = mpq.getTerms();
234234
int[] positions = mpq.getPositions();
235235
SpanQuery[] positionSpanQueries = new SpanQuery[positions.length];
@@ -262,6 +262,8 @@ private Collection<Query> rewriteCustomQuery(Query query) {
262262
// if original slop is 0 then require inOrder
263263
boolean inorder = (mpq.getSlop() == 0);
264264
return Collections.singletonList(new SpanNearQuery(positionSpanQueries, mpq.getSlop() + positionGaps, inorder));
265+
} else if (query instanceof OpenSearchToParentBlockJoinQuery parentQuery) {
266+
return Collections.singletonList(parentQuery.getChildQuery());
265267
} else {
266268
return null;
267269
}

server/src/test/java/org/opensearch/lucene/search/uhighlight/CustomUnifiedHighlighterTests.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.apache.lucene.index.DirectoryReader;
4444
import org.apache.lucene.index.IndexOptions;
4545
import org.apache.lucene.index.IndexWriterConfig;
46+
import org.apache.lucene.index.LeafReaderContext;
4647
import org.apache.lucene.index.Term;
4748
import org.apache.lucene.queries.CommonTermsQuery;
4849
import org.apache.lucene.search.BooleanClause;
@@ -56,13 +57,19 @@
5657
import org.apache.lucene.search.TermQuery;
5758
import org.apache.lucene.search.TopDocs;
5859
import org.apache.lucene.search.highlight.DefaultEncoder;
60+
import org.apache.lucene.search.join.BitSetProducer;
61+
import org.apache.lucene.search.join.ScoreMode;
5962
import org.apache.lucene.search.uhighlight.UnifiedHighlighter;
6063
import org.apache.lucene.store.Directory;
6164
import org.apache.lucene.tests.index.RandomIndexWriter;
65+
import org.apache.lucene.util.BitSet;
66+
import org.apache.lucene.util.FixedBitSet;
6267
import org.opensearch.common.lucene.search.MultiPhrasePrefixQuery;
6368
import org.opensearch.core.common.Strings;
69+
import org.opensearch.index.search.OpenSearchToParentBlockJoinQuery;
6470
import org.opensearch.test.OpenSearchTestCase;
6571

72+
import java.io.IOException;
6673
import java.text.BreakIterator;
6774
import java.util.Locale;
6875

@@ -335,4 +342,33 @@ public void testOverlappingTerms() throws Exception {
335342
assertHighlightOneDoc("text", inputs, analyzer, query, Locale.ROOT, BreakIterator.getSentenceInstance(Locale.ROOT), 0, outputs);
336343
}
337344

345+
public void testOpenSearchToParentBlockJoinQuery() throws Exception {
346+
final String[] inputs = { "Nested highlighting query." };
347+
final String[] outputs = { "Nested <b>highlighting</b> query." };
348+
349+
Query childQuery = new TermQuery(new Term("text", "highlighting"));
350+
BitSetProducer parentsFilter = new BitSetProducer() {
351+
@Override
352+
public BitSet getBitSet(LeafReaderContext context) throws IOException {
353+
FixedBitSet bits = new FixedBitSet(context.reader().maxDoc());
354+
for (int i = 0; i < context.reader().maxDoc(); i++) {
355+
bits.set(i);
356+
}
357+
return bits;
358+
}
359+
};
360+
361+
Query parentQuery = new OpenSearchToParentBlockJoinQuery(childQuery, parentsFilter, ScoreMode.None, "foo");
362+
assertHighlightOneDoc(
363+
"text",
364+
inputs,
365+
new StandardAnalyzer(),
366+
parentQuery,
367+
Locale.ROOT,
368+
BreakIterator.getSentenceInstance(Locale.ROOT),
369+
0,
370+
outputs
371+
);
372+
}
373+
338374
}

0 commit comments

Comments
 (0)