Made sense to me at the time. Still does for what it's worth.
Plus the code using DocumentExtractor sill has to compute and deal with
first and max anyways.
Emmanuel
On Mon 2012-10-29 14:09, Ales Justin wrote:
What's the design decision behind this?
As DE has all the info, it could easily hide the offset (and max) into impl details.
-Ales
On Oct 29, 2012, at 11:14 AM, Emmanuel Bernard <emmanuel(a)hibernate.org> wrote:
> Correct, the DocumentExtractor contract expects the absolute index. It
> looks like a bug in how ISPN's Query module use it.
>
> Emmanuel
>
> On Fri 2012-10-26 16:09, Ales Justin wrote:
>> After searching for the needed in haystack, I finally found the problem.
>> (not to mention complete lack of tests for this *basic* feature ...)
>>
>> The problem is with queries with offset when you iterate over them -- offset is
never taken into account.
>>
>> There are two possible fixes -- as I see them.
>>
>> 1) In HS:
>>
>> DocumentExtractorImpl::extract takes into account "firstIndex"
>>
>> public EntityInfo extract(int scoreDocIndex) throws IOException {
>> int docId = queryHits.docId( firstIndex + scoreDocIndex );
>> Document document = extractDocument( fistIndex + scoreDocIndex );
>>
>> 2) LazyIterator in Infinispan-Query applies the offset:
>>
>> protected EntityInfo loadEntityInfo(int index) {
>> try {
>> return extractor.extract(extractor.getFirstIndex() + index);
>>
>> ---
>>
>> Since those methods are exposed in DocumentExtractor,
>> I would guess they were meant for external code to use them,
>> instead of putting this logic into extractor itself.
>>
>> So, I'll go ahead and provide a patch for (2).
>>
>> -Ales
>>