Hi all,
Thanks for the KIP, Lucia! This is a nice change.
To Kirk's question (1), the example is a bit misleading. The typical
case that would ease user pain is specifically using "null" to indicate
an open-ended range, especially since null is not a valid key.
I could additionally see an empty string as being nice, but the actual
API is generic, not String, so there's no meaningful concept of
empty/blank/whitespace that we could check for, just null or not.
Regarding (2), there's no public factory that takes Optional parameters.
I think you're looking at the private constructor. An alternative Lucia
could consider is to instead propose adding a new factory like
`withRange(Optional<K> lower, Optional<K> upper)`.
FWIW, I'd be in favor of this KIP as proposed.
A couple of smaller notes:
3. In the compatibility notes, I wasn't sure what "web request" was
referring to. I think you just mean that all existing valid API calls
will continue to work the same, and we're only making the withRange
method more permissive with its arguments.
4. For the Test Plan, I wrote some tests that validate these queries
against every kind and configuration of store possible. Please add your
new test cases to that one to make absolutely sure it'll work for every
store. Obviously, you may also want to add some specific unit tests in
addition.
See
https://github.com/apache/kafka/blob/trunk/streams/src/test/java/org/apache/kafka/streams/integration/IQv2StoreIntegrationTest.java
Thanks again!
-John
On 6/21/23 12:00, Kirk True wrote:
Hi Lucia,
One question:
1. Since the proposed implementation change for withRange() method uses
Optional.ofNullable() (which only catches nulls and not blank/whitespace
strings), wouldn’t users still need to have code like that in the example?
2. Why don't users create RangeQuery objects that use Optional directly? What’s
the benefit of introducing what appears to be a very thin utility facade?
Thanks,
Kirk
On Jun 21, 2023, at 9:51 AM, Kirk True <k...@kirktrue.pro> wrote:
Hi Lucia,
Thanks for the KIP!
The KIP wasn’t in the email and I didn’t see it on the main KIP directory. Here
it is:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-941%3A+Range+queries+to+accept+null+lower+and+upper+bounds
Can the KIP be added to the main KIP page
(https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals)?
That will help with discoverability and encourage discussion.
Thanks,
Kirk
On Jun 15, 2023, at 2:13 PM, Lucia Cerchie <lcerc...@confluent.io.INVALID>
wrote:
Hi everyone,
I'd like to discuss KIP-941, which will change the behavior of range
queries to make it easier for users to execute full range scans when using
interactive queries with upper and lower bounds from query parameters in
web client requests.
I much appreciate your input!
Lucia Cerchie
--
[image: Confluent] <https://www.confluent.io>
Lucia Cerchie
Developer Advocate
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: 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>
