You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/09/23 12:21:24 UTC

[GitHub] [lucene] romseygeek opened a new pull request, #11807: No need to rewrite queries in unified highlighter

romseygeek opened a new pull request, #11807:
URL: https://github.com/apache/lucene/pull/11807

   ### Description
   
   Since QueryVisitor added the ability to signal multi-term queries, the query rewrite
   call in UnifiedHighlighter has been essentially useless, and with more aggressive
   rewriting this is now causing bugs like #11490.  We can safely remove this call.
   
   Fixes #11490
   


-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] jpountz commented on pull request #11807: No need to rewrite queries in unified highlighter

Posted by GitBox <gi...@apache.org>.
jpountz commented on PR #11807:
URL: https://github.com/apache/lucene/pull/11807#issuecomment-1380775336

   @romseygeek Should it be backported to branch_9x?


-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] OpakAlex commented on pull request #11807: No need to rewrite queries in unified highlighter

Posted by GitBox <gi...@apache.org>.
OpakAlex commented on PR #11807:
URL: https://github.com/apache/lucene/pull/11807#issuecomment-1274363022

   Thanks for the fix!


-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] romseygeek commented on pull request #11807: No need to rewrite queries in unified highlighter

Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #11807:
URL: https://github.com/apache/lucene/pull/11807#issuecomment-1256931459

   I thought the changes you made for unrecognized queries fixed the issues with the surround query parser? If not it would be good to implement query visitors for the queries that it produces, or at the very least have a test in the highlighting module for them.


-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] dsmiley commented on pull request #11807: No need to rewrite queries in unified highlighter

Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #11807:
URL: https://github.com/apache/lucene/pull/11807#issuecomment-1258920877

   > I thought the changes you made for unrecognized queries fixed the issues with the surround query parser? 
   
   I forget LOL but...
   
   > If not it would be good to implement query visitors for the queries that it produces, or at the very least have a test in the highlighting module for them.
   
   Indeed; let's test & see


-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] romseygeek merged pull request #11807: No need to rewrite queries in unified highlighter

Posted by GitBox <gi...@apache.org>.
romseygeek merged PR #11807:
URL: https://github.com/apache/lucene/pull/11807


-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] romseygeek commented on pull request #11807: No need to rewrite queries in unified highlighter

Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #11807:
URL: https://github.com/apache/lucene/pull/11807#issuecomment-1264427197

   I ported the test in https://issues.apache.org/jira/secure/attachment/12919819/TestUnifiedHighlighterSurround.java to latest main, and I can confirm that it fails if you set `withWeightMatches(false)`.  But! It fails if the empty searcher rewrite is in place as well, so this change makes no difference to it.  So I think we should merge it anyway, as it doesn't seem to break anything that isn't already broken?


-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] dsmiley commented on pull request #11807: No need to rewrite queries in unified highlighter

Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #11807:
URL: https://github.com/apache/lucene/pull/11807#issuecomment-1264749687

   +1 works for me; this change is the right thing to do, even if it surfaces issues that can be separately fixed. 


-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene] romseygeek commented on pull request #11807: No need to rewrite queries in unified highlighter

Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #11807:
URL: https://github.com/apache/lucene/pull/11807#issuecomment-1381532407

   Oops, yes, I should have backported it at the time.  Will do that now!


-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org