Hi, Lucas, Thank you for your suggestions. I will update the KIP and code together.
Sincerely, Hanyu On Tue, Oct 3, 2023 at 8:16 PM Hanyu (Peter) Zheng <pzh...@confluent.io> wrote: > If we use WithDescendingKeys() to generate a RangeQuery to do the > reveseQuery, how do we achieve the methods like withRange, withUpperBound, > and withLowerBound only in this method? > > On Tue, Oct 3, 2023 at 8:01 PM Hanyu (Peter) Zheng <pzh...@confluent.io> > wrote: > >> I believe there's no need to introduce a method like >> WithDescendingKeys(). Instead, we can simply add a reverse flag to >> RangeQuery. Each method within RangeQuery would then accept an additional >> parameter. If the reverse is set to true, it would indicate the results >> should be reversed. >> >> Initially, I introduced a reverse variable. When set to false, the >> RangeQuery class behaves normally. However, when reverse is set to true, >> the RangeQuery essentially takes on the functionality of ReverseRangeQuery. >> Further details can be found in the "Rejected Alternatives" section. >> >> In my perspective, RangeQuery is a class responsible for creating a >> series of RangeQuery objects. It offers methods such as withRange, >> withUpperBound, and withLowerBound, allowing us to generate objects >> representing different queries. I'm unsure how adding a >> withDescendingOrder() method would be compatible with the other methods, >> especially considering that, based on KIP 969, WithDescendingKeys() doesn't >> appear to take any input variables. And if withDescendingOrder() doesn't >> accept any input, how does it return a RangeQuery? >> >> On Tue, Oct 3, 2023 at 4:37 PM Hanyu (Peter) Zheng <pzh...@confluent.io> >> wrote: >> >>> Hi, Colt, >>> The underlying structure of inMemoryKeyValueStore is treeMap. >>> Sincerely, >>> Hanyu >>> >>> On Tue, Oct 3, 2023 at 4:34 PM Hanyu (Peter) Zheng <pzh...@confluent.io> >>> wrote: >>> >>>> Hi Bill, >>>> 1. I will update the KIP in accordance with the PR and synchronize >>>> their future updates. >>>> 2. I will use that name. >>>> 3. you mean add something about ordering at the motivation section? >>>> >>>> Sincerely, >>>> Hanyu >>>> >>>> >>>> On Tue, Oct 3, 2023 at 4:29 PM Hanyu (Peter) Zheng <pzh...@confluent.io> >>>> wrote: >>>> >>>>> Hi, Walker, >>>>> >>>>> 1. I will update the KIP in accordance with the PR and synchronize >>>>> their future updates. >>>>> 2. I will use that name. >>>>> 3. I'll provide additional details in that section. >>>>> 4. I intend to utilize rangeQuery to achieve what we're referring to >>>>> as reverseQuery. In essence, reverseQuery is merely a term. To clear up >>>>> any >>>>> ambiguity, I'll make necessary adjustments to the KIP. >>>>> >>>>> Sincerely, >>>>> Hanyu >>>>> >>>>> >>>>> >>>>> On Tue, Oct 3, 2023 at 4:09 PM Hanyu (Peter) Zheng < >>>>> pzh...@confluent.io> wrote: >>>>> >>>>>> Ok, I will change it back to following the code, and update them >>>>>> together. >>>>>> >>>>>> On Tue, Oct 3, 2023 at 2:27 PM Walker Carlson >>>>>> <wcarl...@confluent.io.invalid> wrote: >>>>>> >>>>>>> Hello Hanyu, >>>>>>> >>>>>>> Looking over your kip things mostly make sense but I have a couple of >>>>>>> comments. >>>>>>> >>>>>>> >>>>>>> 1. You have "withDescandingOrder()". I think you mean >>>>>>> "descending" :) >>>>>>> Also there are still a few places in the do where its called >>>>>>> "setReverse" >>>>>>> 2. Also I like "WithDescendingKeys()" better >>>>>>> 3. I'm not sure of what ordering guarantees we are offering. >>>>>>> Perhaps we >>>>>>> can add a section to the motivation clearly spelling out the >>>>>>> current >>>>>>> ordering and the new offering? >>>>>>> 4. When you say "use unbounded reverseQuery to achieve >>>>>>> reverseAll" do >>>>>>> you mean "use unbounded RangeQuery to achieve reverseAll"? as far >>>>>>> as I can >>>>>>> tell we don't have a reverseQuery as a named object? >>>>>>> >>>>>>> >>>>>>> Looking good so far >>>>>>> >>>>>>> best, >>>>>>> Walker >>>>>>> >>>>>>> On Tue, Oct 3, 2023 at 2:13 PM Colt McNealy <c...@littlehorse.io> >>>>>>> wrote: >>>>>>> >>>>>>> > Hello Hanyu, >>>>>>> > >>>>>>> > Thank you for the KIP. I agree with Matthias' proposal to keep the >>>>>>> naming >>>>>>> > convention consistent with KIP-969. I favor the >>>>>>> `.withDescendingKeys()` >>>>>>> > name. >>>>>>> > >>>>>>> > I am curious about one thing. RocksDB guarantees that records >>>>>>> returned >>>>>>> > during a range scan are lexicographically ordered by the bytes of >>>>>>> the keys >>>>>>> > (either ascending or descending order, as specified in the query). >>>>>>> This >>>>>>> > means that results within a single partition are indeed ordered.** >>>>>>> My >>>>>>> > reading of KIP-805 suggests to me that you don't need to specify >>>>>>> the >>>>>>> > partition number you are querying in IQv2, which means that you >>>>>>> can have a >>>>>>> > valid reversed RangeQuery over a store with "multiple partitions" >>>>>>> in it. >>>>>>> > >>>>>>> > Currently, IQv1 does not guarantee order of keys in this scenario. >>>>>>> Does >>>>>>> > IQv2 support ordering across partitions? Such an implementation >>>>>>> would >>>>>>> > require opening a rocksdb range scan** on multiple rocksdb >>>>>>> instances (one >>>>>>> > per partition), and polling the first key of each. Whether or not >>>>>>> this is >>>>>>> > ordered, could we please add that to the documentation? >>>>>>> > >>>>>>> > **(How is this implemented/guaranteed in an >>>>>>> `inMemoryKeyValueStore`? I >>>>>>> > don't know about that implementation). >>>>>>> > >>>>>>> > Colt McNealy >>>>>>> > >>>>>>> > *Founder, LittleHorse.dev* >>>>>>> > >>>>>>> > >>>>>>> > On Tue, Oct 3, 2023 at 1:35 PM Hanyu (Peter) Zheng >>>>>>> > <pzh...@confluent.io.invalid> wrote: >>>>>>> > >>>>>>> > > ok, I will update it. Thank you Matthias >>>>>>> > > >>>>>>> > > Sincerely, >>>>>>> > > Hanyu >>>>>>> > > >>>>>>> > > On Tue, Oct 3, 2023 at 11:23 AM Matthias J. Sax < >>>>>>> mj...@apache.org> >>>>>>> > wrote: >>>>>>> > > >>>>>>> > > > Thanks for the KIP Hanyu! >>>>>>> > > > >>>>>>> > > > >>>>>>> > > > I took a quick look and it think the proposal makes sense >>>>>>> overall. >>>>>>> > > > >>>>>>> > > > A few comments about how to structure the KIP. >>>>>>> > > > >>>>>>> > > > As you propose to not add `ReverseRangQuery` class, the code >>>>>>> example >>>>>>> > > > should go into "Rejected Alternatives" section, not in the >>>>>>> "Proposed >>>>>>> > > > Changes" section. >>>>>>> > > > >>>>>>> > > > For the `RangeQuery` code example, please omit all existing >>>>>>> methods >>>>>>> > etc, >>>>>>> > > > and only include what will be added/changed. This make it >>>>>>> simpler to >>>>>>> > > > read the KIP. >>>>>>> > > > >>>>>>> > > > >>>>>>> > > > nit: typo >>>>>>> > > > >>>>>>> > > > > the fault value is false >>>>>>> > > > >>>>>>> > > > Should be "the default value is false". >>>>>>> > > > >>>>>>> > > > >>>>>>> > > > Not sure if `setReverse()` is the best name. Maybe >>>>>>> > `withDescandingOrder` >>>>>>> > > > (or similar, I guess `withReverseOrder` would also work) might >>>>>>> be >>>>>>> > > > better? Would be good to align to KIP-969 proposal that >>>>>>> suggest do use >>>>>>> > > > `withDescendingKeys` methods for "reverse key-range"; if we go >>>>>>> with >>>>>>> > > > `withReverseOrder` we should change KIP-969 accordingly. >>>>>>> > > > >>>>>>> > > > Curious to hear what others think about naming this >>>>>>> consistently across >>>>>>> > > > both KIPs. >>>>>>> > > > >>>>>>> > > > >>>>>>> > > > -Matthias >>>>>>> > > > >>>>>>> > > > >>>>>>> > > > On 10/3/23 9:17 AM, Hanyu (Peter) Zheng wrote: >>>>>>> > > > > >>>>>>> > > > >>>>>>> > > >>>>>>> > >>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-985%3A+Add+reverseRange+and+reverseAll+query+over+kv-store+in+IQv2 >>>>>>> > > > > >>>>>>> > > > >>>>>>> > > >>>>>>> > > >>>>>>> > > -- >>>>>>> > > >>>>>>> > > [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> >>>>> >>>> >>>> >>>> -- >>>> >>>> [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> >> > > > -- > > [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>
