On 25 September 2012 09:35, Marko Lukša <[email protected]> wrote: > On 24.9.2012 19:03, Sanne Grinovero wrote: >>> 3. QueryIterator.afterFirst() & beforeLast() >>> These two are OK. Calling next() after afterFirst() should return the >>> second element, while calling previous() should return the first element. >> Are you sure they are ok? I'm actually wondering why they exist at >> all; they don't look like very useful to me. >> Why should someone want a method to directly jump to the second element? >> Users already have the option to "beforeFirst && next" or use the >> "jumpTo" / seek operation. >> > Oh yes, I was wondering about that myself when I first saw > QueryIterator, but then figured someone needed/wanted it. If we > deprecate the whole QueryIterator and introduce a new interface, I'd > leave these methods out. > >>> 4. isFirst(), isLast() >>> ListIterator does not have the concept of "current element", but the >>> javadoc of all the is*() methods talk about the current element, which >>> is ambiguous. Therefore isFirst() and isLast() should probably be >>> renamed to isBeforeFirst() and isAfterLast(). >> I would remove these too. Am I missing some use case? >> > I think you're right. Also, instead of calling isBeforeFirst() and > isAfterLast(), users can simply call !hasPrevious() and !hasNext(), > which already exist. > >>> 5. jumpToResult(int index) >>> When jumping to index=1, what does next() return? What about previous()? >>> We should probably replace the method with two others: beforeResult(int >>> index) and afterResult(int index) >> I would add only "beforeResult(int)", again I don't see why we should >> have both versions. >> Since you have to do "next()" to retrieve a value, I'd consider >> "beforeResults" more user friendly than "afterResults" > Hmm, what if you want to iterate backwards. If you only had > beforeResult(int), you wouldn't be able to jump behind the last element. > You'd have to use afterLast(). For usecases where you always jump to the > end and iterate backwards, afterLast() would be ok. But what if you > needed to implement a method like iterateBackwards(int fromIndex, > Visitor visitor). Then the method would need to have something like: if > (fromIndex == col.size()) it.afterLast() else it.beforeResult(fromIndex). > > Of course, we could allow users to call beforeResult(col.size()), but > that's not completely clean, since there's no element with index > col.size(). But still, it may be cleaner than having an additional > method (afterResult).
Right I hadn't thought about backwards scrolling. Don't you think that being able to revert the sort order at query level is enough? If someone needs A-Z, runs the scroll in forward mode and defines lexicographical ordering, if you then need Z-A you revert the sort and run the iterator again in forward only mode: AFAIR supporting the reverse iterator in distributed lazy queries is a nightmare, so if we agree they are pointless that's a good simplification in the codebase. I can think of a single use case in which one can not revert the Sort: when the search result is defined by relevance in a fulltext query, but in this case the revert sort doesn't make any sense as the last results (which would get on top) are a "tail" of uninteresting information. > > >>> Any comments / other ideas? >>> >>> Marko >> That's a lot of changes needed. One would say we could deprecate all >> methods and rewrite the new ones, but wouldn't it be better to >> deprecate the interface and introduce a brand new one? >> What about "ResultsIterator" ? after all, we're not iterating on queries.. > > I agree. The only thing that bothers me is that CacheQuery would then > need new methods called (lazy)resultIterator() (instead of iterator(), > which already exists). Right. though I like "resultIterator()" more so we're having luck in this case ;) Any better suggestion for names? Anyone thinks it's worth to remove the old ones without a deprecation step to reuse the old names? > > This reminds me. What is the difference between LazyIterator and > EagerIterator? Both of them execute the Lucene query immediately. The > only difference is that EagerIterator loads EntityInfos straight away, > while LazyIterator (only) loads the DocumentExtractor and then at first > invocation of next(), it loads the EntityInfo and cache value at the > same time (when filling the buffer). > > IMO, the difference between them is negligible and we could either scrap > LazyResultIterator or make EagerResultIterator preload everything? You have to consider that Infinispan could hold many more values than what the current node/JVM is able to keep in memory; for example when running a distributed query, it's likely that the user doesn't want to preload all data; obviously when running "list()" we *could* load things lazily on access, but that gets tricky as we don't have a close() method and I would expect people to use an iterator if they know the amount of data retrieved is having some critical mass. Of course if you know you're going to hit a limited number of entries (and these entries are of limited size - some people store hd movie streams) then the eager loading could be more efficient. So the lazy-loaders are needed; it could be nice to think of a different way to specify the option; we could have something like .iterate(LoadMode); rather than having to define the methods twice. Sanne > > > Marko > _______________________________________________ > infinispan-dev mailing list > [email protected] > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/infinispan-dev
