Upgrade to lucene-5.2.0-snapshot-1673124. #10562
Conversation
|
LGTM, I like the added checks in AllTermSpanScorer. |
| @@ -51,7 +51,7 @@ public AllTermQuery(Term term) { | |||
| } | |||
|
|
|||
| @Override | |||
| public Weight createWeight(IndexSearcher searcher, boolean needsScores) throws IOException { | |||
| public SpanWeight createWeight(IndexSearcher searcher, boolean needsScores) throws IOException { | |||
There was a problem hiding this comment.
This class overrides weight/scorer, where is its positions check (throw IAE if proximity is not enabled) ?
There was a problem hiding this comment.
we need to fix it in Lucene too then
There was a problem hiding this comment.
Maybe you are a bit behind. I added tests for this position check last night and fixed bugs in the default impl.
|
I actually disagree with mike. Paul went the route of adding these asserts to every single spans, and yet they could still not be debugged. Problem is: when trying to duplicate checks like this across N classes then its inevitable that some will be incomplete. you are also bound to have some buggy asserts (false fails) and it also makes it really hard to read the code. So I think the asserts should be removed (from lucene too). I opened https://issues.apache.org/jira/browse/LUCENE-6411 yesterday and worked on it all day long to fix the root cause. |
|
While I do agree that AssertingSpans is a better way to find bugs, all that this patch does is aligning AllTermQuery with SpanTermQuery from which it is derived. I can push a new update once LUCENE-6411 is done. |
|
LUCENE-6411 is already done. Please don't add these redundant asserts. it just causes me more work to remove them. |
|
I can remove them but then you might want to remove them from SpanScorer.setFreqCurrentDoc too. |
|
The only reason they are still there is because i needed to get some amount of sleep last night. It is work to cleanup all this stuff. |
|
See my commit, i cleaned up this query: it doesn't manipulate Spans anymore. |
|
Thanks @rmuir ! |
…1673124 Upgrade to lucene-5.2.0-snapshot-1673124.
clintongormley
added
:Core/Infra/Core
No description provided.