-
Notifications
You must be signed in to change notification settings - Fork 769
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lucene 10.0.0 #4570
base: master
Are you sure you want to change the base?
lucene 10.0.0 #4570
Conversation
public BitIntsHolder reduce(Collection<SuggestResultCollector> collectors) { | ||
BitIntsHolder reduced = documentIds; | ||
for (SuggestResultCollector collector : collectors) { | ||
documentIds.or(collector.documentIds); //TODO fix as per https://github.com/apache/lucene/pull/766/files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs fixing
seems SuggestResultCollector.java has no testing coverage ... |
@ahornace any clues for proper fixing of reduce for that collector? Is it even used? |
@@ -205,18 +205,17 @@ private void searchMultiDatabase(List<Project> projectList, boolean paging) thro | |||
} | |||
|
|||
private void searchIndex(IndexSearcher searcher, boolean paging) throws IOException { | |||
collector = TopScoreDocCollector.create(hitsPerPage * cachePages, Short.MAX_VALUE); | |||
collectorManager = new TopScoreDocCollectorManager(hitsPerPage * cachePages, Short.MAX_VALUE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps redesign to reuse the object?
(supress-es should be reviewed, scorer returns bad objects, so it should break scorer tests)
lucene 10 support needs to be rewritten into new PR |
179fdb9
to
4374b79
Compare
4374b79
to
7834538
Compare
url.getHost(), url.getPort(), | ||
url.getPath(), url.getQuery(), url.getRef()); | ||
public static String encodeURL(String urlStr) throws URISyntaxException { | ||
// URL url = new URL(urlStr); - this call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a longer look at the departure from the URL()
constructors yesterday. The URL
class by itself does not support any encoding/decoding, it merely breaks down the URL into pieces. The encoding is then supplied by the URI
class.
The problem is that URI(String)
accepts only already encoded URIs and refuses any non-compliant characters. I think the way to go is to propagate the URL
object all the way where possible, i.e. change the String url
parameter type of the various linkify()
and buildLink()
methods to URL url
and avoid passing the url in the map of attributes as String
- pass it as an extra argument with the URL
type.
It is still not clear to me where the unescaped characters could originate from.
No description provided.