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>
