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>



Reply via email to