Thanks, Hanyu, for the KIP and all the updates. I just do not understand the purpose of defining new time ranges (`newTimeFrom`, `newTimeTo`). Why don't we simply re-use the existing time range variables?
Bests, Alieh On Thu, Nov 30, 2023 at 8:34 PM Hanyu (Peter) Zheng <pzh...@confluent.io.invalid> wrote: > new KIP link: > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-997%3A++update+WindowRangeQuery+and+unify+WindowKeyQuery+and+WindowRangeQuery > > On Wed, Nov 29, 2023 at 10:12 PM Hanyu (Peter) Zheng <pzh...@confluent.io> > wrote: > > > Thank you Bruno, > > 1. Thank you for the notification. I have updated the ticket link > > accordingly. > > 2. Certainly, I'll update the KIP name. Should I initiate a new > discussion > > for it, because if I change the name, the link will change. > > 3. Understood, I will add that to the KIP. > > 4. I propose we accept both > > `WindowRangeQuery.withAllKeys().fromTime(time1).toTime(time2)` and > > `WindowRangeQuery.withKeyRange(key1, > key2).fromTime(time1).toTime(time2)`, > > while also reusing the existing `withKey` method. > > 5. Following a discussion with Matthias, we've decided to defer the > > implementation of order guarantees to a future KIP. > > > > Sincerely, > > Hanyu > > > > On Wed, Nov 29, 2023 at 6:22 AM Bruno Cadonna <cado...@apache.org> > wrote: > > > >> Hi, > >> > >> Thanks for the updates! > >> > >> > >> 1. > >> Could you please link the correct ticket in the KIP? > >> > >> 2. > >> Could you please adapt the motivation section and the title to the > >> updated goal of the KIP? There is no fetch() or fetchAll() method in the > >> query class. > >> > >> 3. > >> Could you please add the "// newly added" comment to all parts that were > >> newly added? That is methods lowerKeyBound() and upperKeyBound(). > >> > >> 4. > >> We should use a more fluent API as I proposed in my last e-mail: > >> > >> Here again > >> > >> WindowRangeQuery.withAllKeys().fromTime(time1).toTime(time2); > >> WindowRangeQuery.withKey(key1).fromTime(time1).toTime(time2); > >> WindowRangeQuery.withKeyRange(key1, key2).fromTime(time1).toTime(time2); > >> > >> 5. > >> We should also consider the order of the results similar as we did in > >> KIP-968. Alternatively, we do not guarantee any order and postpone the > >> order guarantees to a future KIP. > >> > >> > >> Best, > >> Bruno > >> > >> > >> > >> On 11/17/23 3:11 AM, Matthias J. Sax wrote: > >> > Thanks for the KIP. > >> > > >> > Given how `WindowRangeQuery` works right now, it's really time to > >> > improve it. > >> > > >> > > >> > 1) Agree. It's not clear what will be added right now. I think we > >> should > >> > deprecate existing `getKey()` w/o an actually replacement? For > >> > `getFromKey` and `getToKey` we should actually be `lowerKeyBound()` > and > >> > `upperKeyBound()` to align to KIP-969? > >> > > >> > Also wondering if we should deprecate existing `withKey()` and > >> > `withWindowStartRange`? `withKey` only works for SessionStores and > >> > implements a single-key full-time-range query. Similarly, > >> > `withWindowStartRange` only works for WindowedStore and implements an > >> > all-key time-range query. Thus, both are rather special and it seems > >> the > >> > aim of this KIP is to generalize `WindowRangeQuery` to arbitrary > >> > key-range/time-range queries? > >> > > >> > What raises one question about time-range semantics, given that we > >> query > >> > windows with different semantics. > >> > > >> > - The current `WindowStore` semantics used for > >> > `WindowRangeQuery#withWindowStartRange` is considering only the window > >> > start time, ie, the window-start time must fall into the query > >> > time-range to be returned. > >> > > >> > - In contrast, `SessionStore` time ranges base on `findSession` use > >> > earliest-session-end-time and latest-session-end-time and thus > >> implement > >> > an "window-bounds / search-time-range overlap query". > >> > > >> > Is there any concern about semantic differences? I would also be > >> > possible to use the same semantics for both query types, and maybe > even > >> > let the user pick with semantics they want (let users chose might > >> > actually be the best thing to do)? -- We can also do this > >> incrementally, > >> > and limit the scope of this KIP (or keep the full KIP scope but > >> > implement it incrementally only) > >> > > >> > Btw: I think we should not add any ordering at this point, and > >> > explicitly state that no ordering is guarantee whatsoever at this > point. > >> > > >> > > >> > > >> > 2) Agreed. We should deprecate `getFromTime` and `getToTime` and add > >> new > >> > method `fromTime` and `toTime`. > >> > > >> > > >> > > >> > 3) About the API. If we move forward with general key-range/time-range > >> I > >> > agree that a more modular approach might be nice. Not sure right now, > >> > what the best approach would be for this? Looking into KIP-969, we > >> might > >> > want to have: > >> > > >> > - static withKeyRange > >> > - static withLowerKeyBound > >> > - static withUpperKeyBound > >> > - static withAllKeys (KIP-969 actually uses `allKeys` ?) > >> > - fromTime > >> > - toTime > >> > > >> > with default-time range would be "all / unbounded" ? > >> > > >> > > >> > > >> > 10: you mentioned that `WindowKeyQuery` functionality can be covered > by > >> > `WindowRangeQuery`. I agree. For this case, it seems we want to > >> > deprecate `WindowKeyQuery` entirely? > >> > > >> > > >> > > >> > -Matthias > >> > > >> > On 11/16/23 1:19 AM, Bruno Cadonna wrote: > >> >> Hi Hanyu, > >> >> > >> >> Thanks for the KIP! > >> >> > >> >> 1) > >> >> Could you please mark the pieces that you want to add to the API in > >> >> the code listing in the KIP? You can add a comment like "// newly > >> >> added" or similar. That would make reading the KIP a bit easier > >> >> because one does not need to compare your code with the code in the > >> >> current codebase. > >> >> > >> >> 2) > >> >> Could you -- as a side cleanup -- also change the getters to not use > >> >> the get-prefix anymore, please? That was apparently an oversight when > >> >> those methods were added. Although the API is marked as Evolving, I > >> >> think we should still deprecate the getX() methods, since it does not > >> >> cost us anything. > >> >> > >> >> 3) > >> >> I propose to make the API a bit more fluent. For example, something > >> like > >> >> > >> >> WindowRangeQuery.withKey(key).fromTime(t1).toTime(t2) > >> >> > >> >> and > >> >> > >> >> WindowRangeQuery.withAllKeys().fromTime(t1).toTime(t2) > >> >> > >> >> and > >> >> > >> >> WindowRangeQuery.withKeyRange(key1, key2).fromTime(t1).toTime(t2) > >> >> > >> >> and maybe even in addition to the above add also the option to start > >> >> with the time range > >> >> > >> >> WindowRangeQuery.withWindowStartRange(t1, > t2).fromKey(key1).toKey(key2) > >> >> > >> >> > >> >> 4) > >> >> Could you also add some usage examples? Alieh did quite a nice job > >> >> regarding usage examples in KIP-986. > >> >> > >> >> > >> >> Best, > >> >> Bruno > >> >> > >> >> On 11/8/23 8:02 PM, Hanyu (Peter) Zheng wrote: > >> >>> Hello everyone, > >> >>> > >> >>> I would like to start the discussion for KIP-997: Support > >> fetch(fromKey, > >> >>> toKey, from, to) to WindowRangeQuery and unify WindowKeyQuery and > >> >>> WindowRangeQuery > >> >>> The KIP can be found here: > >> >>> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-997%3A++Support+fetch%28fromKey%2C+toKey%2C+from%2C+to%29+to+WindowRangeQuery+and+unify+WindowKeyQuery+and+WindowRangeQuery > >> >>> > >> >>> Any suggestions are more than welcome. > >> >>> > >> >>> Many thanks, > >> >>> Hanyu > >> >>> > >> >>> On Wed, Nov 8, 2023 at 10:38 AM Hanyu (Peter) Zheng > >> >>> <pzh...@confluent.io> > >> >>> wrote: > >> >>> > >> >>>> > >> >>>> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-997%3A++Support+fetch%28fromKey%2C+toKey%2C+from%2C+to%29+to+WindowRangeQuery+and+unify+WindowKeyQuery+and+WindowRangeQuery > >> >>>> > >> >>>> -- > >> >>>> > >> >>>> [image: Confluent] <https://www.confluent.io> > >> >>>> Hanyu (Peter) Zheng he/him/his > >> >>>> Software Engineer Intern > >> >>>> +1 (213) 431-7193 <+1+(213)+431-7193> > >> >>>> Follow us: [image: Blog] > >> >>>> < > >> > https://www.confluent.io/blog?utm_source=footer&utm_medium=email&utm_campaign=ch.email-signature_type.community_content.blog > >> >[image: > >> >>>> Twitter] <https://twitter.com/ConfluentInc>[image: LinkedIn] > >> >>>> <https://www.linkedin.com/in/hanyu-peter-zheng/>[image: Slack] > >> >>>> <https://slackpass.io/confluentcommunity>[image: YouTube] > >> >>>> <https://youtube.com/confluent> > >> >>>> > >> >>>> [image: Try Confluent Cloud for Free] > >> >>>> < > >> > https://www.confluent.io/get-started?utm_campaign=tm.fm-apac_cd.inbound&utm_source=gmail&utm_medium=organic > >> > > >> >>>> > >> >>> > >> >>> > >> > > > > > > -- > > > > [image: Confluent] <https://www.confluent.io> > > Hanyu (Peter) Zheng he/him/his > > Software Engineer Intern > > +1 (213) 431-7193 <+1+(213)+431-7193> > > Follow us: [image: Blog] > > < > https://www.confluent.io/blog?utm_source=footer&utm_medium=email&utm_campaign=ch.email-signature_type.community_content.blog > >[image: > > Twitter] <https://twitter.com/ConfluentInc>[image: LinkedIn] > > <https://www.linkedin.com/in/hanyu-peter-zheng/>[image: Slack] > > <https://slackpass.io/confluentcommunity>[image: YouTube] > > <https://youtube.com/confluent> > > > > [image: Try Confluent Cloud for Free] > > < > https://www.confluent.io/get-started?utm_campaign=tm.fm-apac_cd.inbound&utm_source=gmail&utm_medium=organic > > > > > > > -- > > [image: Confluent] <https://www.confluent.io> > Hanyu (Peter) Zheng he/him/his > Software Engineer Intern > +1 (213) 431-7193 <+1+(213)+431-7193> > Follow us: [image: Blog] > < > https://www.confluent.io/blog?utm_source=footer&utm_medium=email&utm_campaign=ch.email-signature_type.community_content.blog > >[image: > Twitter] <https://twitter.com/ConfluentInc>[image: LinkedIn] > <https://www.linkedin.com/in/hanyu-peter-zheng/>[image: Slack] > <https://slackpass.io/confluentcommunity>[image: YouTube] > <https://youtube.com/confluent> > > [image: Try Confluent Cloud for Free] > < > https://www.confluent.io/get-started?utm_campaign=tm.fm-apac_cd.inbound&utm_source=gmail&utm_medium=organic > > >
