Thank you Alieh, After discussion with Matthias, we decide use oldTimeFrom and oldTimeTo(these two will Deprecated soom) to implement withWindowStartRange() and use timeFrom and timeTo to implement withAllKey() and withKeyRange(), I will update the KIP.
Sincerely, Hanyu On Sun, Dec 3, 2023 at 3:11 PM Alieh Saeedi <asae...@confluent.io.invalid> wrote: > 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 > > > > > > -- [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>
