[ 
https://issues.apache.org/jira/browse/LUCENE-2878?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13064380#comment-13064380
 ] 

Simon Willnauer commented on LUCENE-2878:
-----------------------------------------

bq.  actually I might suggest renaming PositionIntervalIterator.collect() to 
distribute(), to distinguish it from its counterpart, 
PositionCollector.collect(). 
how about gatherPositions() ?

bq. Do you have any concern about the two iterators getting out of sync? I 
noticed the nocommit, I guess that's what you meant?
Actually I am not super concerned about this. its all up to the API consumer. 
The nocommit is just a reminder that we need to fix this method (PII#doc()) to 
return the actual document the DocsAndPositionsEnum points to or rather the 
iterator points to right now. I think we should start sketching out the API and 
write some javadoc to make clear how things work. Beside working on 
highlighting I think we should also cut over remaining queries to positions and 
copy some of the span tests to positions (dedicated issue for this would be 
helpful this gets a little big).

bq. should we be thinking about making it impossible for the user of the API 
get themselves in trouble? Say, for example, I call advanceTo(randomDocID) - I 
could cause my PositionFilterQuery to get out of whack, maybe?

phew, I think we can work around this but we need to make sure we don't loose 
flexibility. Maybe we need to rethink how PositionFitlerQuery works. Lets leave 
that for later :)

For spans I think we should move them to the new queries module and eventually 
out of core (we should have a new issue for this no?). For the position 
iterating stuff I think we can mainly concentrate on getting positions work and 
leave payloads for later. 

Further I think we should also open a ticket for highlighting as well as for 
positional scoring where we can add the 2 stage collector stuff etc.

I will create a "positions branch" version so we can flag issues correctly. 

bq. I am going to clean up the PosHighlighter tests a bit, get rid of dead 
code, etc., possibly add some tests for the composite interval stuff, and do a 
little benchmarking.

awesome, if you clean up the patch make sure we have the right headers in all 
new files and add @lucene.experimental to the classes. I want to commit our 
stage soonish (once you cleaned it up) and continue with fine grained issues. 

I am glad that you spend so much time this man! Making positions first class 
citizens is very important and it will pave the way to get rid of spans 
eventually. 
  

> Allow Scorer to expose positions and payloads aka. nuke spans 
> --------------------------------------------------------------
>
>                 Key: LUCENE-2878
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2878
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: core/search
>    Affects Versions: Bulk Postings branch
>            Reporter: Simon Willnauer
>            Assignee: Simon Willnauer
>              Labels: gsoc2011, lucene-gsoc-11, mentor
>         Attachments: LUCENE-2878-OR.patch, LUCENE-2878.patch, 
> LUCENE-2878.patch, LUCENE-2878.patch, LUCENE-2878.patch, LUCENE-2878.patch, 
> LUCENE-2878.patch, LUCENE-2878.patch, LUCENE-2878_trunk.patch, 
> LUCENE-2878_trunk.patch, PosHighlighter.patch, PosHighlighter.patch
>
>
> Currently we have two somewhat separate types of queries, the one which can 
> make use of positions (mainly spans) and payloads (spans). Yet Span*Query 
> doesn't really do scoring comparable to what other queries do and at the end 
> of the day they are duplicating lot of code all over lucene. Span*Queries are 
> also limited to other Span*Query instances such that you can not use a 
> TermQuery or a BooleanQuery with SpanNear or anthing like that. 
> Beside of the Span*Query limitation other queries lacking a quiet interesting 
> feature since they can not score based on term proximity since scores doesn't 
> expose any positional information. All those problems bugged me for a while 
> now so I stared working on that using the bulkpostings API. I would have done 
> that first cut on trunk but TermScorer is working on BlockReader that do not 
> expose positions while the one in this branch does. I started adding a new 
> Positions class which users can pull from a scorer, to prevent unnecessary 
> positions enums I added ScorerContext#needsPositions and eventually 
> Scorere#needsPayloads to create the corresponding enum on demand. Yet, 
> currently only TermQuery / TermScorer implements this API and other simply 
> return null instead. 
> To show that the API really works and our BulkPostings work fine too with 
> positions I cut over TermSpanQuery to use a TermScorer under the hood and 
> nuked TermSpans entirely. A nice sideeffect of this was that the Position 
> BulkReading implementation got some exercise which now :) work all with 
> positions while Payloads for bulkreading are kind of experimental in the 
> patch and those only work with Standard codec. 
> So all spans now work on top of TermScorer ( I truly hate spans since today ) 
> including the ones that need Payloads (StandardCodec ONLY)!!  I didn't bother 
> to implement the other codecs yet since I want to get feedback on the API and 
> on this first cut before I go one with it. I will upload the corresponding 
> patch in a minute. 
> I also had to cut over SpanQuery.getSpans(IR) to 
> SpanQuery.getSpans(AtomicReaderContext) which I should probably do on trunk 
> first but after that pain today I need a break first :).
> The patch passes all core tests 
> (org.apache.lucene.search.highlight.HighlighterTest still fails but I didn't 
> look into the MemoryIndex BulkPostings API yet)

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to