[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-11-02 Thread Timothy055
Github user Timothy055 commented on the issue: https://github.com/apache/lucene-solr/pull/105 Ah, missed that. Fixed --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-11-02 Thread dsmiley
Github user dsmiley commented on the issue: https://github.com/apache/lucene-solr/pull/105 Nevermind on `MultiPostingsEnum`. MPE assumes the PostingsEnum are for a disjoint set of documents -- i.e. each PostingsEnum matches documents disjoint from the others. This makes sense given

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-11-02 Thread Timothy055
Github user Timothy055 commented on the issue: https://github.com/apache/lucene-solr/pull/105 Fair enough on both ends. And yes, we have seen a potential use case for a unified view over terms. I'll take a look at MultiPostingsEnum and see if we can use that. --- If your project

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-11-02 Thread dsmiley
Github user dsmiley commented on the issue: https://github.com/apache/lucene-solr/pull/105 _(I wrote this as you made your last comments, but rather than delete it I'll comment any way)_ The documentation for `PostingsEnum.nextPosition()` states that calling it more than

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-11-02 Thread Timothy055
Github user Timothy055 commented on the issue: https://github.com/apache/lucene-solr/pull/105 Merged your commit. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-11-02 Thread Timothy055
Github user Timothy055 commented on the issue: https://github.com/apache/lucene-solr/pull/105 Forget it, you can't do that because the next position on one enum might be -1, but there's more enums left in the queue so the user of this class could inadvertently terminate early if

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-11-02 Thread Timothy055
Github user Timothy055 commented on the issue: https://github.com/apache/lucene-solr/pull/105 Hmm, I know we're already knowingly breaking the PostingsEnum contract, but rather than throwing a IllegaStateException, maybe we could still return positions, they just happen to be

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-10-29 Thread dsmiley
Github user dsmiley commented on the issue: https://github.com/apache/lucene-solr/pull/105 Check it out: https://github.com/dsmiley/lucene-solr/commit/50e2ea89c7eb15c863aa6e04e14fd32085ee85bd Remember this is a package-private class internal to the UnifiedHighlighter. Not

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-10-29 Thread Timothy055
Github user Timothy055 commented on the issue: https://github.com/apache/lucene-solr/pull/105 Hmm, clever! But not sure I find it very clean though. I feel like that can lead to trouble down the road if code ever expects the offsets to be ordered. If we went that route we wouldn't

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-10-29 Thread dsmiley
Github user dsmiley commented on the issue: https://github.com/apache/lucene-solr/pull/105 I started playing with a bit and realized the same thing. It looks straight-forward but it's deceptively more complicated. Then it hit me -- lets not try to return the correct position at

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-10-29 Thread Timothy055
Github user Timothy055 commented on the issue: https://github.com/apache/lucene-solr/pull/105 I don't think there's a way to avoid keeping the position state, unfortunately. The reason is that we can move one of the postings enums to the next position, but then realize the next

[GitHub] lucene-solr issue #105: LUCENE-7526 Improvements to UnifiedHighlighter Offse...

2016-10-28 Thread Timothy055
Github user Timothy055 commented on the issue: https://github.com/apache/lucene-solr/pull/105 I've pushed some more changes now. Still taking a look at what we might be able to do further with CompositePostingsEnum --- If your project is set up for it, you can reply to this email