Thanks for splitting this part into a separate KIP!

For `withKey()` we should be explicit that `null` is not allowed.

(Looking into existing `KeyQuery` it seems the JavaDocs don't cover this either -- would you like to do a tiny cleanup PR for this, or fix on-the-side in one of your PRs?)



The key query returns all the records that are valid in the time range starting 
from the timestamp {@code fromTimestamp}.

In the JavaDocs you use the phrase `are valid` -- I think we need to explain what "valid" means? It might even be worth to add some examples. It's annoying, but being precise if kinda important.

With regard to KIP-962, should we allow `null` for time bounds ? The JavaDocs should also be explicit if `null` is allowed or not and what the semantics are if allowed.



You are using `asOf()` however, because we are doing time-range queries, to me using `until()` to describe the upper bound would sound better (I am not a native speaker though, so maybe I am off?)


The key query returns all the records that have timestamp <= {@code 
asOfTimestamp}.

This is only correct if not lower-bound is set, right?


In your reply to KIP-960 you mentioned:

the meaningless combinations are prevented by throwing exceptions.

We should add corresponding JavaDocs like:

   @throws IllegalArgumentException if {@code fromTimestamp} is equal or
                                    larger than {@code untilTimestamp}

Or something similar.


With regard to KIP-960: if we need to introduce a `VersionedKeyQuery` class for single-key-single-ts lookup, would we need to find a new name for the query class of this KIP, given that the return type is different?


-Matthias



On 8/16/23 10:57 AM, Alieh Saeedi wrote:
Hi all,

I splitted KIP-960
<https://cwiki.apache.org/confluence/display/KAFKA/KIP-960%3A+Support+single-key_single-timestamp+interactive+queries+%28IQv2%29+for+versioned+state+stores>
into three separate KIPs. Therefore, please continue discussions
about single-key, multi-timestamp interactive queries here. You can see all
the addressed reviews on the following page. Thanks in advance.

KIP-968: Support single-key_multi-timestamp interactive queries (IQv2) for
versioned state stores
<https://cwiki.apache.org/confluence/display/KAFKA/KIP-968%3A+Support+single-key_multi-timestamp+interactive+queries+%28IQv2%29+for+versioned+state+stores>

I look forward to your feedback!

Cheers,
Alieh

Reply via email to