Thank you for the feedback Michal. While I agree the return may be a little bit more confusing to reason about, the reason for doing so was to keep the range query interfaces consistent with their single-key counterparts.
In the case of the window store, the "key" of the single-key iterator is the actual timestamp of the underlying entry, not just range of the window, so if we were to wrap the result key a window we wouldn't be getting back the equivalent of the single key iterator. In both cases peekNextKey is just returning the timestamp of the next entry in the window store that matches the query. In the case of the session store, we already return Windowed<K> for the single-key method, so it made sense there to also return Windowed<K> for the range method. Hope this make sense? Let me know if you still have concerns about this. Thank you, Xavier On Wed, May 10, 2017 at 12:25 PM Michal Borowiecki < michal.borowie...@openbet.com> wrote: > Apologies, I missed the discussion (or lack thereof) about the return > type of: > > WindowStoreIterator<KeyValue<K, V>> fetch(K from, K to, long timeFrom, > long timeTo) > > > WindowStoreIterator<V> (as the KIP mentions) is a subclass of > KeyValueIterator<Long, V> > > KeyValueIterator<K,V> has the following method: > > /** * Peek at the next key without advancing the iterator * @return the > key of the next value that would be returned from the next call to next > */ K peekNextKey(); > > Given the type in this case will be Long, I assume what it would return > is the window timestamp of the next found record? > > > In the case of WindowStoreIterator<V> fetch(K key, long timeFrom, long > timeTo); > all records found by fetch have the same key, so it's harmless to return > the timestamp of the next found window but here we have varying keys and > varying windows, so won't it be too confusing? > > KeyValueIterator<Windowed<K>, V> (as in the proposed > ReadOnlySessionStore.fetch) just feels much more intuitive. > > Apologies again for jumping onto this only once the voting has already > begun. > Thanks, > Michał > > On 10/05/17 20:08, Sriram Subramanian wrote: > > +1 > > > > On Wed, May 10, 2017 at 11:42 AM, Bill Bejeck <bbej...@gmail.com> wrote: > > > >> +1 > >> > >> Thanks, > >> Bill > >> > >> On Wed, May 10, 2017 at 2:38 PM, Guozhang Wang <wangg...@gmail.com> > wrote: > >> > >>> +1. Thank you! > >>> > >>> On Wed, May 10, 2017 at 11:30 AM, Xavier Léauté <xav...@confluent.io> > >>> wrote: > >>> > >>>> Hi everyone, > >>>> > >>>> Since there aren't any objections to this addition, I would like to > >> start > >>>> the voting on KIP-155 so we can hopefully get this into 0.11. > >>>> > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+ > >>>> 155+-+Add+range+scan+for+windowed+state+stores > >>>> > >>>> Voting will stay active for at least 72 hours. > >>>> > >>>> Thank you, > >>>> Xavier > >>>> > >>> > >>> > >>> -- > >>> -- Guozhang > >>> > >
