apanimesh061 commented on a change in pull request #362:
URL: https://github.com/apache/lucene/pull/362#discussion_r730555792
##########
File path:
lucene/highlighter/src/java/org/apache/lucene/search/uhighlight/UnifiedHighlighter.java
##########
@@ -1168,9 +1174,12 @@ public CacheHelper getReaderCacheHelper() {
/**
* Internally use the {@link Weight#matches(LeafReaderContext, int)} API
for highlighting. It's
- * more accurate to the query, though might not calculate passage
relevancy as well. Use of this
- * flag requires {@link #MULTI_TERM_QUERY} and {@link #PHRASES}. {@link
- * #PASSAGE_RELEVANCY_OVER_SPEED} will be ignored. False by default.
+ * more accurate to the query, and the snippets can be a little different
for phrases because
+ * the whole phrase is marked up instead of each word. The passage
relevancy calculation can be
+ * different (maybe worse?) and it's slower when highlighting many fields.
Use of this flag
+ * requires {@link #MULTI_TERM_QUERY} and {@link #PHRASES}. {@link
+ * #PASSAGE_RELEVANCY_OVER_SPEED} will be ignored. True by default, so
long as the requirements
Review comment:
Thanks for confirming.
Based on reading the current code
[here](https://github.com/apache/lucene/blob/175a49e54a231db5ba07d8997b55a3faa9806bae/lucene/highlighter/src/java/org/apache/lucene/search/uhighlight/UnifiedHighlighter.java#L828-L836),
it looks like `shouldPreferPassageRelevancyOverSpeed` will always return true
by default since `defaultPassageRelevancyOverSpeed ` is
[true](https://github.com/apache/lucene/blob/175a49e54a231db5ba07d8997b55a3faa9806bae/lucene/highlighter/src/java/org/apache/lucene/search/uhighlight/UnifiedHighlighter.java#L125)
by default. This should mean that `highlightFlags` will contain
`HighlightFlag.PASSAGE_RELEVANCY_OVER_SPEED` by default.
So the code change I made:
```
if (highlightFlags.contains(HighlightFlag.MULTI_TERM_QUERY)
&& highlightFlags.contains(HighlightFlag.PHRASES)
&& !highlightFlags.contains(HighlightFlag.PASSAGE_RELEVANCY_OVER_SPEED))
{
highlightFlags.add(HighlightFlag.WEIGHT_MATCHES);
}
```
will not work since `PASSAGE_RELEVANCY_OVER_SPEED` will exist as a flag and
`WEIGHT_MATCHES` will not be enabled. I think in that case it should be:
```
if (highlightFlags.contains(HighlightFlag.MULTI_TERM_QUERY) &&
highlightFlags.contains(HighlightFlag.PHRASES)) {
highlightFlags.add(HighlightFlag.WEIGHT_MATCHES);
}
```
It is possible that I am misunderstanding something here since I am new to
highlighters.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
