You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "zhaih (via GitHub)" <gi...@apache.org> on 2023/11/06 06:39:25 UTC
[PR] Enable executing using NFA in RegexpQuery [lucene]
zhaih opened a new pull request, #12767:
URL: https://github.com/apache/lucene/pull/12767
### Description
<!--
If this is your first contribution to Lucene, please make sure you have reviewed the contribution guide.
https://github.com/apache/lucene/blob/main/CONTRIBUTING.md
-->
As title, added a new flag in `RegexpQuery`'s ctor, not sure whether there's a better way, maybe using a static method instead? Like `RegexpQuery.createWithDFA`/`RegexpQuery.createWithNFA`?
--
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
Re: [PR] Enable executing using NFA in RegexpQuery [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on PR #12767:
URL: https://github.com/apache/lucene/pull/12767#issuecomment-1794689085
There should be tests exercising this new boolean option. How do we know it gives correct results to do this?
--
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
Re: [PR] Enable executing using NFA in RegexpQuery [lucene]
Posted by "zhaih (via GitHub)" <gi...@apache.org>.
zhaih merged PR #12767:
URL: https://github.com/apache/lucene/pull/12767
--
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
Re: [PR] Enable executing using NFA in RegexpQuery [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on code in PR #12767:
URL: https://github.com/apache/lucene/pull/12767#discussion_r1384795478
##########
lucene/core/src/test/org/apache/lucene/search/TestRegexpQuery.java:
##########
@@ -80,7 +80,10 @@ private long caseInsensitiveRegexQueryNrHits(String regex) throws IOException {
newTerm(regex),
RegExp.ALL,
RegExp.ASCII_CASE_INSENSITIVE,
- Operations.DEFAULT_DETERMINIZE_WORK_LIMIT);
+ RegexpQuery.DEFAULT_PROVIDER,
+ Operations.DEFAULT_DETERMINIZE_WORK_LIMIT,
+ MultiTermQuery.CONSTANT_SCORE_BLENDED_REWRITE,
+ random().nextBoolean());
Review Comment:
yeah unfortunately this test is incredibly weak...
--
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
Re: [PR] Enable executing using NFA in RegexpQuery [lucene]
Posted by "rmuir (via GitHub)" <gi...@apache.org>.
rmuir commented on PR #12767:
URL: https://github.com/apache/lucene/pull/12767#issuecomment-1794787497
I was thinking just to exercise it in TestRegexpRandom2 or similar test. Maybe add TestRegexpRandom3.java?
Test is conceptually very simple but powerful. It has a lot of lines of code only because it finds bugs in terms dictionaries etc too :)
--
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
Re: [PR] Enable executing using NFA in RegexpQuery [lucene]
Posted by "zhaih (via GitHub)" <gi...@apache.org>.
zhaih commented on code in PR #12767:
URL: https://github.com/apache/lucene/pull/12767#discussion_r1387529653
##########
lucene/core/src/test/org/apache/lucene/search/TestRegexpQuery.java:
##########
@@ -80,7 +80,10 @@ private long caseInsensitiveRegexQueryNrHits(String regex) throws IOException {
newTerm(regex),
RegExp.ALL,
RegExp.ASCII_CASE_INSENSITIVE,
- Operations.DEFAULT_DETERMINIZE_WORK_LIMIT);
+ RegexpQuery.DEFAULT_PROVIDER,
+ Operations.DEFAULT_DETERMINIZE_WORK_LIMIT,
+ MultiTermQuery.CONSTANT_SCORE_BLENDED_REWRITE,
+ random().nextBoolean());
Review Comment:
OK, I added the NFA version to `TestRegexpRandom2`
--
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
Re: [PR] Enable executing using NFA in RegexpQuery [lucene]
Posted by "zhaih (via GitHub)" <gi...@apache.org>.
zhaih commented on code in PR #12767:
URL: https://github.com/apache/lucene/pull/12767#discussion_r1384459245
##########
lucene/core/src/test/org/apache/lucene/search/TestRegexpQuery.java:
##########
@@ -80,7 +80,10 @@ private long caseInsensitiveRegexQueryNrHits(String regex) throws IOException {
newTerm(regex),
RegExp.ALL,
RegExp.ASCII_CASE_INSENSITIVE,
- Operations.DEFAULT_DETERMINIZE_WORK_LIMIT);
+ RegexpQuery.DEFAULT_PROVIDER,
+ Operations.DEFAULT_DETERMINIZE_WORK_LIMIT,
+ MultiTermQuery.CONSTANT_SCORE_BLENDED_REWRITE,
+ random().nextBoolean());
Review Comment:
@rmuir I added this line so it will be tested randomly, and this method actually is widely used in the RegexpQuery's tests.
But for sure I can (will) add a dedicated test to test NFA with random regex.
--
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