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>
