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 2019/12/23 15:55:38 UTC
[GitHub] [lucene-solr] andywebb1975 opened a new pull request #1113:
SOLR-14131: adds maxQueryLength option
andywebb1975 opened a new pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113
# Description
Attempting to spellcheck some long query terms can trigger org.apache.lucene.util.automaton.TooComplexToDeterminizeException.
# Solution
This change (previously discussed in SOLR-13190, and dependent on LUCENE-9102) adds a maxQueryLength option to DirectSolrSpellChecker so that Lucene/Solr can be configured to not attempt to spellcheck terms over a specified length.
# Tests
A new test checks that a term is spellchecked before maxQueryLength is reduced, and not afterwards.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `master` branch.
- [ ] I have run `ant precommit` and the appropriate test suite.
- [x] I have added tests for my changes.
- [x] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] bruno-roustant commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
bruno-roustant commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360949352
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -88,6 +88,45 @@ public void testOnlyMorePopularWithExtendedResults() throws Exception {
"//lst[@name='spellcheck']/lst[@name='suggestions']/lst[@name='fox']/arr[@name='suggestion']/lst/int[@name='freq']=2",
"//lst[@name='spellcheck']/bool[@name='correctlySpelled']='true'"
);
- }
+ }
+
+ @Test
+ public void testMaxQueryLength() throws Exception {
+ testMaxQueryLength(true);
+ testMaxQueryLength(false);
+ }
+
+ private void testMaxQueryLength(Boolean limitQueryLength) throws Exception {
+
+ DirectSolrSpellChecker checker = new DirectSolrSpellChecker();
+ NamedList spellchecker = new NamedList();
+ spellchecker.add("classname", DirectSolrSpellChecker.class.getName());
+ spellchecker.add(SolrSpellChecker.FIELD, "teststop");
+ spellchecker.add(DirectSolrSpellChecker.MINQUERYLENGTH, 2);
+
+ // demonstrate that "anothar" is not corrected when maxQueryLength is set to a small number
+ if (limitQueryLength) spellchecker.add(DirectSolrSpellChecker.MAXQUERYLENGTH, 4);
+
+ SolrCore core = h.getCore();
+ checker.init(spellchecker, core);
+
+ h.getCore().withSearcher(searcher -> {
+ Collection<Token> tokens = queryConverter.convert("anothar");
+ SpellingOptions spellOpts = new SpellingOptions(tokens, searcher.getIndexReader());
+ SpellingResult result = checker.getSuggestions(spellOpts);
+ assertTrue("result should not be null", result != null);
+ Map<String, Integer> suggestions = result.get(tokens.iterator().next());
+ assertTrue("suggestions should not be null", suggestions != null);
Review comment:
assertNotNull?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] bruno-roustant commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
bruno-roustant commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360950592
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -79,7 +79,7 @@ public void test() throws Exception {
return null;
});
}
-
+
Review comment:
(comment race)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] bruno-roustant commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
bruno-roustant commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360949198
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -88,6 +88,45 @@ public void testOnlyMorePopularWithExtendedResults() throws Exception {
"//lst[@name='spellcheck']/lst[@name='suggestions']/lst[@name='fox']/arr[@name='suggestion']/lst/int[@name='freq']=2",
"//lst[@name='spellcheck']/bool[@name='correctlySpelled']='true'"
);
- }
+ }
+
+ @Test
+ public void testMaxQueryLength() throws Exception {
+ testMaxQueryLength(true);
+ testMaxQueryLength(false);
+ }
+
+ private void testMaxQueryLength(Boolean limitQueryLength) throws Exception {
+
+ DirectSolrSpellChecker checker = new DirectSolrSpellChecker();
+ NamedList spellchecker = new NamedList();
Review comment:
We should use generics here: NamedList<Object> spellchecker = new NamedList<>();
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] bruno-roustant commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
bruno-roustant commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360950705
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -88,6 +88,45 @@ public void testOnlyMorePopularWithExtendedResults() throws Exception {
"//lst[@name='spellcheck']/lst[@name='suggestions']/lst[@name='fox']/arr[@name='suggestion']/lst/int[@name='freq']=2",
"//lst[@name='spellcheck']/bool[@name='correctlySpelled']='true'"
);
- }
+ }
+
+ @Test
+ public void testMaxQueryLength() throws Exception {
+ testMaxQueryLength(true);
+ testMaxQueryLength(false);
+ }
+
+ private void testMaxQueryLength(Boolean limitQueryLength) throws Exception {
+
+ DirectSolrSpellChecker checker = new DirectSolrSpellChecker();
+ NamedList spellchecker = new NamedList();
+ spellchecker.add("classname", DirectSolrSpellChecker.class.getName());
+ spellchecker.add(SolrSpellChecker.FIELD, "teststop");
+ spellchecker.add(DirectSolrSpellChecker.MINQUERYLENGTH, 2);
+
+ // demonstrate that "anothar" is not corrected when maxQueryLength is set to a small number
+ if (limitQueryLength) spellchecker.add(DirectSolrSpellChecker.MAXQUERYLENGTH, 4);
+
+ SolrCore core = h.getCore();
+ checker.init(spellchecker, core);
+
+ h.getCore().withSearcher(searcher -> {
+ Collection<Token> tokens = queryConverter.convert("anothar");
+ SpellingOptions spellOpts = new SpellingOptions(tokens, searcher.getIndexReader());
+ SpellingResult result = checker.getSuggestions(spellOpts);
+ assertTrue("result should not be null", result != null);
+ Map<String, Integer> suggestions = result.get(tokens.iterator().next());
+ assertTrue("suggestions should not be null", suggestions != null);
+
+ if (limitQueryLength) {
+ assertTrue("suggestions should be empty", suggestions.isEmpty());
+ } else {
+ Map.Entry<String, Integer> entry = suggestions.entrySet().iterator().next();
+ assertTrue(entry.getKey() + " is not equal to 'another'", entry.getKey().equals("another") == true);
Review comment:
(comment race)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] bruno-roustant commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
bruno-roustant commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360949293
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -88,6 +88,45 @@ public void testOnlyMorePopularWithExtendedResults() throws Exception {
"//lst[@name='spellcheck']/lst[@name='suggestions']/lst[@name='fox']/arr[@name='suggestion']/lst/int[@name='freq']=2",
"//lst[@name='spellcheck']/bool[@name='correctlySpelled']='true'"
);
- }
+ }
+
+ @Test
+ public void testMaxQueryLength() throws Exception {
+ testMaxQueryLength(true);
+ testMaxQueryLength(false);
+ }
+
+ private void testMaxQueryLength(Boolean limitQueryLength) throws Exception {
+
+ DirectSolrSpellChecker checker = new DirectSolrSpellChecker();
+ NamedList spellchecker = new NamedList();
+ spellchecker.add("classname", DirectSolrSpellChecker.class.getName());
+ spellchecker.add(SolrSpellChecker.FIELD, "teststop");
+ spellchecker.add(DirectSolrSpellChecker.MINQUERYLENGTH, 2);
+
+ // demonstrate that "anothar" is not corrected when maxQueryLength is set to a small number
+ if (limitQueryLength) spellchecker.add(DirectSolrSpellChecker.MAXQUERYLENGTH, 4);
+
+ SolrCore core = h.getCore();
+ checker.init(spellchecker, core);
+
+ h.getCore().withSearcher(searcher -> {
+ Collection<Token> tokens = queryConverter.convert("anothar");
+ SpellingOptions spellOpts = new SpellingOptions(tokens, searcher.getIndexReader());
+ SpellingResult result = checker.getSuggestions(spellOpts);
+ assertTrue("result should not be null", result != null);
Review comment:
assertNotNull?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] bruno-roustant commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
bruno-roustant commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360947790
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -79,7 +79,7 @@ public void test() throws Exception {
return null;
});
}
-
+
Review comment:
Good point. Indeed the test should be fixed line 77 to use spellOpts.tokens instead and expect empty suggestions.
Would you like to fix it?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] andywebb1975 commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
andywebb1975 commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360930320
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -79,7 +79,7 @@ public void test() throws Exception {
return null;
});
}
-
+
Review comment:
It's not clear to me what the "super" test above is for. As far as I can see, the test runs a spellcheck for "super" but then uses "fob" as the index into suggestions, which will never find an entry.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] andywebb1975 commented on issue #1113: SOLR-14131:
adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
andywebb1975 commented on issue #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#issuecomment-568568657
Thanks Bruno and Mike - I've submitted some updates, could you take another look please?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] andywebb1975 commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
andywebb1975 commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360989706
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -79,7 +79,7 @@ public void test() throws Exception {
return null;
});
}
-
+
Review comment:
I think it's clearer what's going on there 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] bruno-roustant commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
bruno-roustant commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360949433
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -88,6 +88,45 @@ public void testOnlyMorePopularWithExtendedResults() throws Exception {
"//lst[@name='spellcheck']/lst[@name='suggestions']/lst[@name='fox']/arr[@name='suggestion']/lst/int[@name='freq']=2",
"//lst[@name='spellcheck']/bool[@name='correctlySpelled']='true'"
);
- }
+ }
+
+ @Test
+ public void testMaxQueryLength() throws Exception {
+ testMaxQueryLength(true);
+ testMaxQueryLength(false);
+ }
+
+ private void testMaxQueryLength(Boolean limitQueryLength) throws Exception {
+
+ DirectSolrSpellChecker checker = new DirectSolrSpellChecker();
+ NamedList spellchecker = new NamedList();
+ spellchecker.add("classname", DirectSolrSpellChecker.class.getName());
+ spellchecker.add(SolrSpellChecker.FIELD, "teststop");
+ spellchecker.add(DirectSolrSpellChecker.MINQUERYLENGTH, 2);
+
+ // demonstrate that "anothar" is not corrected when maxQueryLength is set to a small number
+ if (limitQueryLength) spellchecker.add(DirectSolrSpellChecker.MAXQUERYLENGTH, 4);
+
+ SolrCore core = h.getCore();
+ checker.init(spellchecker, core);
+
+ h.getCore().withSearcher(searcher -> {
+ Collection<Token> tokens = queryConverter.convert("anothar");
+ SpellingOptions spellOpts = new SpellingOptions(tokens, searcher.getIndexReader());
+ SpellingResult result = checker.getSuggestions(spellOpts);
+ assertTrue("result should not be null", result != null);
+ Map<String, Integer> suggestions = result.get(tokens.iterator().next());
+ assertTrue("suggestions should not be null", suggestions != null);
+
+ if (limitQueryLength) {
+ assertTrue("suggestions should be empty", suggestions.isEmpty());
+ } else {
+ Map.Entry<String, Integer> entry = suggestions.entrySet().iterator().next();
+ assertTrue(entry.getKey() + " is not equal to 'another'", entry.getKey().equals("another") == true);
Review comment:
assertEquals
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1113:
SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360938182
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -79,7 +79,7 @@ public void test() throws Exception {
return null;
});
}
-
+
Review comment:
Yes, and it asserts that there are no results. It's the negative test case for the spell checking match.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] asfgit closed pull request #1113: SOLR-14131: adds
maxQueryLength option
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] bruno-roustant commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
bruno-roustant commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360949962
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -88,6 +88,45 @@ public void testOnlyMorePopularWithExtendedResults() throws Exception {
"//lst[@name='spellcheck']/lst[@name='suggestions']/lst[@name='fox']/arr[@name='suggestion']/lst/int[@name='freq']=2",
"//lst[@name='spellcheck']/bool[@name='correctlySpelled']='true'"
);
- }
+ }
+
+ @Test
+ public void testMaxQueryLength() throws Exception {
+ testMaxQueryLength(true);
+ testMaxQueryLength(false);
+ }
+
+ private void testMaxQueryLength(Boolean limitQueryLength) throws Exception {
+
+ DirectSolrSpellChecker checker = new DirectSolrSpellChecker();
+ NamedList spellchecker = new NamedList();
+ spellchecker.add("classname", DirectSolrSpellChecker.class.getName());
+ spellchecker.add(SolrSpellChecker.FIELD, "teststop");
+ spellchecker.add(DirectSolrSpellChecker.MINQUERYLENGTH, 2);
+
+ // demonstrate that "anothar" is not corrected when maxQueryLength is set to a small number
+ if (limitQueryLength) spellchecker.add(DirectSolrSpellChecker.MAXQUERYLENGTH, 4);
+
+ SolrCore core = h.getCore();
+ checker.init(spellchecker, core);
+
+ h.getCore().withSearcher(searcher -> {
+ Collection<Token> tokens = queryConverter.convert("anothar");
+ SpellingOptions spellOpts = new SpellingOptions(tokens, searcher.getIndexReader());
+ SpellingResult result = checker.getSuggestions(spellOpts);
+ assertTrue("result should not be null", result != null);
+ Map<String, Integer> suggestions = result.get(tokens.iterator().next());
+ assertTrue("suggestions should not be null", suggestions != null);
+
+ if (limitQueryLength) {
+ assertTrue("suggestions should be empty", suggestions.isEmpty());
+ } else {
+ Map.Entry<String, Integer> entry = suggestions.entrySet().iterator().next();
Review comment:
Maybe we could insert an assertFalse(suggestions.isEmpty()) otherwise the line below will throw a NoSuchElementException less nice in a test.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1113:
SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360934384
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -88,6 +88,45 @@ public void testOnlyMorePopularWithExtendedResults() throws Exception {
"//lst[@name='spellcheck']/lst[@name='suggestions']/lst[@name='fox']/arr[@name='suggestion']/lst/int[@name='freq']=2",
"//lst[@name='spellcheck']/bool[@name='correctlySpelled']='true'"
);
- }
+ }
+
+ @Test
+ public void testMaxQueryLength() throws Exception {
+ testMaxQueryLength(true);
+ testMaxQueryLength(false);
+ }
+
+ private void testMaxQueryLength(Boolean limitQueryLength) throws Exception {
+
+ DirectSolrSpellChecker checker = new DirectSolrSpellChecker();
+ NamedList spellchecker = new NamedList();
+ spellchecker.add("classname", DirectSolrSpellChecker.class.getName());
+ spellchecker.add(SolrSpellChecker.FIELD, "teststop");
+ spellchecker.add(DirectSolrSpellChecker.MINQUERYLENGTH, 2);
+
+ // demonstrate that "anothar" is not corrected when maxQueryLength is set to a small number
+ if (limitQueryLength) spellchecker.add(DirectSolrSpellChecker.MAXQUERYLENGTH, 4);
+
+ SolrCore core = h.getCore();
+ checker.init(spellchecker, core);
+
+ h.getCore().withSearcher(searcher -> {
+ Collection<Token> tokens = queryConverter.convert("anothar");
+ SpellingOptions spellOpts = new SpellingOptions(tokens, searcher.getIndexReader());
+ SpellingResult result = checker.getSuggestions(spellOpts);
+ assertTrue("result should not be null", result != null);
Review comment:
minor nit: we can get cleaner test failures by using `assertNotNull` here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] andywebb1975 commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
andywebb1975 commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360989610
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -88,6 +88,45 @@ public void testOnlyMorePopularWithExtendedResults() throws Exception {
"//lst[@name='spellcheck']/lst[@name='suggestions']/lst[@name='fox']/arr[@name='suggestion']/lst/int[@name='freq']=2",
"//lst[@name='spellcheck']/bool[@name='correctlySpelled']='true'"
);
- }
+ }
+
+ @Test
+ public void testMaxQueryLength() throws Exception {
+ testMaxQueryLength(true);
+ testMaxQueryLength(false);
+ }
+
+ private void testMaxQueryLength(Boolean limitQueryLength) throws Exception {
+
+ DirectSolrSpellChecker checker = new DirectSolrSpellChecker();
+ NamedList spellchecker = new NamedList();
+ spellchecker.add("classname", DirectSolrSpellChecker.class.getName());
+ spellchecker.add(SolrSpellChecker.FIELD, "teststop");
+ spellchecker.add(DirectSolrSpellChecker.MINQUERYLENGTH, 2);
+
+ // demonstrate that "anothar" is not corrected when maxQueryLength is set to a small number
+ if (limitQueryLength) spellchecker.add(DirectSolrSpellChecker.MAXQUERYLENGTH, 4);
+
+ SolrCore core = h.getCore();
+ checker.init(spellchecker, core);
+
+ h.getCore().withSearcher(searcher -> {
+ Collection<Token> tokens = queryConverter.convert("anothar");
+ SpellingOptions spellOpts = new SpellingOptions(tokens, searcher.getIndexReader());
+ SpellingResult result = checker.getSuggestions(spellOpts);
+ assertTrue("result should not be null", result != null);
+ Map<String, Integer> suggestions = result.get(tokens.iterator().next());
+ assertTrue("suggestions should not be null", suggestions != null);
+
+ if (limitQueryLength) {
+ assertTrue("suggestions should be empty", suggestions.isEmpty());
+ } else {
+ Map.Entry<String, Integer> entry = suggestions.entrySet().iterator().next();
Review comment:
done!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] madrob commented on a change in pull request #1113:
SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360934621
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -88,6 +88,45 @@ public void testOnlyMorePopularWithExtendedResults() throws Exception {
"//lst[@name='spellcheck']/lst[@name='suggestions']/lst[@name='fox']/arr[@name='suggestion']/lst/int[@name='freq']=2",
"//lst[@name='spellcheck']/bool[@name='correctlySpelled']='true'"
);
- }
+ }
+
+ @Test
+ public void testMaxQueryLength() throws Exception {
+ testMaxQueryLength(true);
+ testMaxQueryLength(false);
+ }
+
+ private void testMaxQueryLength(Boolean limitQueryLength) throws Exception {
+
+ DirectSolrSpellChecker checker = new DirectSolrSpellChecker();
+ NamedList spellchecker = new NamedList();
+ spellchecker.add("classname", DirectSolrSpellChecker.class.getName());
+ spellchecker.add(SolrSpellChecker.FIELD, "teststop");
+ spellchecker.add(DirectSolrSpellChecker.MINQUERYLENGTH, 2);
+
+ // demonstrate that "anothar" is not corrected when maxQueryLength is set to a small number
+ if (limitQueryLength) spellchecker.add(DirectSolrSpellChecker.MAXQUERYLENGTH, 4);
+
+ SolrCore core = h.getCore();
+ checker.init(spellchecker, core);
+
+ h.getCore().withSearcher(searcher -> {
+ Collection<Token> tokens = queryConverter.convert("anothar");
+ SpellingOptions spellOpts = new SpellingOptions(tokens, searcher.getIndexReader());
+ SpellingResult result = checker.getSuggestions(spellOpts);
+ assertTrue("result should not be null", result != null);
+ Map<String, Integer> suggestions = result.get(tokens.iterator().next());
+ assertTrue("suggestions should not be null", suggestions != null);
+
+ if (limitQueryLength) {
+ assertTrue("suggestions should be empty", suggestions.isEmpty());
+ } else {
+ Map.Entry<String, Integer> entry = suggestions.entrySet().iterator().next();
+ assertTrue(entry.getKey() + " is not equal to 'another'", entry.getKey().equals("another") == true);
Review comment:
minor nit: use `assertEquals` here
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] andywebb1975 commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
andywebb1975 commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360990276
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -88,6 +88,45 @@ public void testOnlyMorePopularWithExtendedResults() throws Exception {
"//lst[@name='spellcheck']/lst[@name='suggestions']/lst[@name='fox']/arr[@name='suggestion']/lst/int[@name='freq']=2",
"//lst[@name='spellcheck']/bool[@name='correctlySpelled']='true'"
);
- }
+ }
+
+ @Test
+ public void testMaxQueryLength() throws Exception {
+ testMaxQueryLength(true);
+ testMaxQueryLength(false);
+ }
+
+ private void testMaxQueryLength(Boolean limitQueryLength) throws Exception {
+
+ DirectSolrSpellChecker checker = new DirectSolrSpellChecker();
+ NamedList spellchecker = new NamedList();
Review comment:
I've just used what you suggested here - am not too familiar with how this works.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] andywebb1975 commented on a change in pull request
#1113: SOLR-14131: adds maxQueryLength option
Posted by GitBox <gi...@apache.org>.
andywebb1975 commented on a change in pull request #1113: SOLR-14131: adds maxQueryLength option
URL: https://github.com/apache/lucene-solr/pull/1113#discussion_r360930320
##########
File path: solr/core/src/test/org/apache/solr/spelling/DirectSolrSpellCheckerTest.java
##########
@@ -79,7 +79,7 @@ public void test() throws Exception {
return null;
});
}
-
+
Review comment:
It's not clear to me what the "super" test above does. As far as I can see, the test runs a spellcheck for "super" but then uses "fob" as the index into suggestions, which will never find an entry.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org