Thanks, Matthias and Bruno, for the feedback on KIP-969. Here is a summary
of the updates I made to the KIP:

   1.  I liked the idea of renaming methods as Matthias suggested.
   2. I removed the allversions() method as I did in KIP-968. To retrieve
   the latest value(s), the user must call just the asOf method with the MAX
   value (asOf(MAX)). The same applies to KIP-968. Do you think it is clumsy,
   Matthias?
   3. I added a method to the *VersionedKeyValueStore *interface, as I did
   for KIP-968.
   4. Matthias: I do not get what you mean by your second comment. Isn't
   the KIP already explicit about that?

   > I assume, results are returned by timestamp for each key. The KIP
   should be explicit about it.


Cheers,
Alieh



On Tue, Oct 3, 2023 at 6:07 AM Matthias J. Sax <mj...@apache.org> wrote:

> Thanks for updating the KIP.
>
> Not sure if I agree or not with Bruno's idea to split the query types
> further? In the end, we split them only because there is three different
> return types: single value, value-iterator, key-value-iterator.
>
> What do we gain by splitting out single-ts-range-key? In the end, for
> range-ts-range-key the proposed class is necessary and is a superset
> (one can set both timestamps to the same value, for single-ts lookup).
>
> The mentioned simplification might apply to "single-ts-range-key" but I
> don't see a simplification for the proposed (and necessary) query type?
>
> On the other hand, I see an advantage of a single-ts-range-key for
> querying over the "latest version" with a range of keys. For a
> single-ts-range-key query, this it would be the default (similar to
> VersionedKeyQuery with not asOf-timestamped defined).
>
> In the current version of the KIP, (if we agree that default should
> actually return "all versions" not "latest" -- this default was
> suggested by Bruno on KIP-968 and makes sense to me, so we would need to
> have the same default here to stay consistent), users would need to pass
> in `from(Long.MAX).to(Long.MAX)` (if I got this right) to query the
> latest point in time only, what seems to be clumsy? Or we could add a
> `lastestKeyOnly` option to `MultiVersionedRangeQuery`, but it does seems
> a little clumsy, too.
>
>
>
> > The overall order of the returned records is by Key
>
> I assume, results are returned by timestamp for each key. The KIP should
> be explicit about it.
>
>
>
> To be very explicit, should we rename the methods to specify the key bound?
>
>   - withRange -> withKeyRange
>   - withLowerBound -> withLowerKeyBound
>   - withUpperBound -> withUpperKeyBound
>   - withNoBounds -> allKeys (or withNoKeyBounds, but we use
> `allVersions` and not `noTimeBound` and should align the naming?)
>
>
>
> -Matthias
>
>
> On 9/6/23 5:25 AM, Bruno Cadonna wrote:
> > Hi Alieh,
> >
> > Thanks for the KIP!
> >
> > One high level comment/question:
> >
> > I assume you separated single key queries into two classes because
> > versioned key queries return a single value and multi version key
> > queries return iterators. Although, range queries always return
> > iterators, it would make sense to also separate range queries for
> > versioned state stores into range queries that return one single version
> > of the keys within a range and range queries that return multiple
> > version of the keys within a range, IMO. That would reduce the
> > meaningless combinations.
> > WDYT?
> >
> > Best,
> > Bruno
> >
> > On 8/16/23 8:01 PM, 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 range
> >> interactive queries here. You can see all the addressed reviews on the
> >> following page. Thanks in advance.
> >>
> >> KIP-969: Support range interactive queries (IQv2) for versioned state
> >> stores
> >> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-969%3A+Support+range+interactive+queries+%28IQv2%29+for+versioned+state+stores
> >
> >>
> >> I look forward to your feedback!
> >>
> >> Cheers,
> >> Alieh
> >>
>

Reply via email to