apanimesh061 commented on a change in pull request #362:
URL: https://github.com/apache/lucene/pull/362#discussion_r730190396



##########
File path: 
lucene/highlighter/src/java/org/apache/lucene/search/uhighlight/UnifiedHighlighter.java
##########
@@ -86,7 +86,7 @@
  *   <li>{@link #getFormatter(String)}: Customize how snippets are formatted.
  * </ul>
  *
- * <p>This is thread-safe.
+ * <p>This is thread-safe, notwithstanding the setters.

Review comment:
       Hi, yes I'll be working on it this weekend. Sorry was super busy last 
week.

##########
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:
       According to this line, it looks like `WEIGHT_MATCHES` is applicable 
only when `MULTI_TERM_QUERY` and `PHRASES` are enabled. And it does not matter 
if `PASSAGE_RELEVANCY_OVER_SPEED` enabled or not. Is my understanding correct?
   
   I am asking this since I can see that by default `defaultHandleMtq`, 
`defaultHighlightPhrasesStrictly` and `defaultPassageRelevancyOverSpeed` are 
`true` which would mean `WEIGHT_MATCHES` will never be applied based on the 
logic I had added.

##########
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:
       According to this line, it looks like `WEIGHT_MATCHES` is applicable 
only when `MULTI_TERM_QUERY` and `PHRASES` are enabled. And it does not matter 
if `PASSAGE_RELEVANCY_OVER_SPEED` enabled or not. Is my understanding correct?
   
   I am asking this since I can see that by default `defaultHandleMtq`, 
`defaultHighlightPhrasesStrictly` and `defaultPassageRelevancyOverSpeed` are 
`true` which would mean `WEIGHT_MATCHES` will never be applied based on the 
logic I had added. This also means that `WEIGHT_MATCHES` is not enabled by 
default?
   
   Is `PASSAGE_RELEVANCY_OVER_SPEED` supposed to be marked as `false` by 
default?

##########
File path: 
lucene/highlighter/src/java/org/apache/lucene/search/uhighlight/UnifiedHighlighter.java
##########
@@ -86,7 +86,7 @@
  *   <li>{@link #getFormatter(String)}: Customize how snippets are formatted.
  * </ul>
  *
- * <p>This is thread-safe.
+ * <p>This is thread-safe, notwithstanding the setters.

Review comment:
       Hi, yes I'll be working on it this weekend. I did not get a chance to 
work on it last week.




-- 
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

Reply via email to