apanimesh061 commented on a change in pull request #362:
URL: https://github.com/apache/lucene/pull/362#discussion_r734200431
##########
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:
> I added your changes but made 3 edits:
>
> * Removed your change to the randomized highlighter configuration. It
was working before; didn't need anything. Thus we want to continue to test
with WEIGHT_MATCHES being off, even when the other settings allow for it to be
enabled.
>
> * Thanks to the test, which failed, I realized the boolean for
checking PASSAGE_RELEVANCY_OVER_SPEED was inverted.
>
> * Enhanced the added test to check how many enum values there are so
that if we change these enums, we intentionally revisit the default assertions.
>
>
> This looks ready to me.
@dsmiley Thanks a lot for fixing that. I was not sure if the tests were
supposed to fail.
Just for clarification, maybe I misunderstood your earlier comments. My
understanding was that WEIGHT_MATCHES should be enabled when MULTI_TERM_QUERY
and PHRASES are enabled and it does not matter if PASSAGE_RELEVANCY_OVER_SPEED
is enabled. Based on your modification, it looks like all 3 should be enabled
for the WEIGHT_MATCHES to be enabled?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
